debbie failing on PostgreSQL 10 run on topology

[robe]

Debbie started failing a day or so ago on her PostgreSQL 10 run.

I suspect it may be a change upstream because her 11 is fine and Bessie 64-bit 10 is also fine.

She follows 10 branch, so they may have accidentally introduced a breaking change:

 regress/topoelementarray_agg .. ok 
 regress/topogeo_addlinestring .. failed (diff expected obtained: /var/lib/jenkins/workspace/postgis/tmp/2_5_pg10w64/test_29_diff)
-----------------------------------------------------------------------------
--- regress/topogeo_addlinestring_expected	2017-12-27 17:51:25.767418144 +0000
+++ /var/lib/jenkins/workspace/postgis/tmp/2_5_pg10w64/test_29_out	2018-06-26 18:41:30.907981957 +0000
@@ -185,8 +185,7 @@
 t3280.start|t
 t3280|L11
 t3280|L22
-t3280|L1b4
-t3280|L1b2
+ERROR:  ORDER/GROUP BY expression not found in targetlist
 t3280.end|Topology 'bug3280' dropped
 t3380.start|t
 t3380.L1|1
-----------------------------------------------------------------------------
 regress/topogeo_addpoint .. ok 
 regress/topogeo_addpolygon .. ok 

[robe]

Seems to have started failing after this commit:

    Add PGTYPESchar_free() to avoid cross-module problems on Windows. (details)

Commit 3566873f21909397561418fab22c98332fa8b72a by tmunro
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=3566873f21909397561418fab22c98332fa8b72a

Add PGTYPESchar_free() to avoid cross-module problems on Windows.
On Windows, it is sometimes important for corresponding malloc() and 
free() calls to be made from the same DLL, since some build options can 
result in multiple allocators being active at the same time.  For that 
reason we already provided PQfreemem().  This commit adds a similar 
function for freeing string results allocated by the pgtypes library.
Author: Takayuki Tsunakawa Reviewed-by: Kyotaro Horiguchi Discussion:
https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8AD5D6%40G01JPEXMBYT05

The file was modified	src/interfaces/ecpg/test/pgtypeslib/dt_test.pgc
The file was modified	src/interfaces/ecpg/test/expected/pgtypeslib-dt_test.c
The file was modified	src/interfaces/ecpg/test/expected/pgtypeslib-num_test2.c
The file was modified	src/interfaces/ecpg/pgtypeslib/common.c
The file was modified	src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc
The file was modified	doc/src/sgml/ecpg.sgml
The file was modified	src/interfaces/ecpg/test/sql/sqlda.pgc
The file was modified	src/interfaces/ecpg/test/pgtypeslib/num_test2.pgc
The file was modified	src/interfaces/ecpg/include/pgtypes_interval.h
The file was modified	src/interfaces/ecpg/test/expected/pgtypeslib-num_test.c
The file was modified	src/interfaces/ecpg/include/pgtypes_timestamp.h
The file was modified	src/interfaces/ecpg/include/pgtypes_numeric.h
The file was modified	src/interfaces/ecpg/test/expected/preproc-outofscope.c
The file was modified	src/interfaces/ecpg/include/Makefile
The file was modified	src/interfaces/ecpg/include/pgtypes_date.h
The file was modified	src/interfaces/ecpg/pgtypeslib/exports.txt
The file was added	src/interfaces/ecpg/include/pgtypes.h
The file was modified	src/interfaces/ecpg/test/expected/pgtypeslib-dt_test2.c
The file was modified	src/interfaces/ecpg/test/pgtypeslib/num_test.pgc
The file was modified	src/interfaces/ecpg/test/expected/sql-sqlda.c

I'll report upstream

[robe]

Reported on pgsql-hackers

​https://www.postgresql.org/message-id/001501d40f88%2475186950%245f493bf0%24%40pcorp.us

[robe]

I was mistaken about the commit, David Rowley, pointed out it sounds more like this one:

​https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=a4c95b0b80c70677c09c0d5c82a6fba875160288

Fix mishandling of sortgroupref labels while splitting SRF targetlists.

split_pathtarget_at_srfs() neglected to worry about sortgroupref labels
in the intermediate PathTargets it constructs.  I think we'd supposed
that their labeling didn't matter, but it does at least for the case that
GroupAggregate/GatherMerge nodes appear immediately under the ProjectSet
step(s).  This results in "ERROR: ORDER/GROUP BY expression not found in
targetlist" during create_plan(), as reported by Rajkumar Raghuwanshi.

To fix, make this logic track the sortgroupref labeling of expressions,
not just their contents.  This also restores the pre-v10 behavior that
separate GROUP BY expressions will be kept distinct even if they are
textually equal().

Discussion: https://postgr.es/m/CAKcux6=1_Ye9kx8YLBPmJs_xE72PPc6vNi5q2AOHowMaCWjJ2w@mail.gmail.com

The test we have which triggers it is badly written so we should probably change it anyway as it's using a set returning function in the SELECT (topogeo_AddLineString) and ordering by a constant which doesn't even guarantee ORDER, so why we even have to order by 1 there is kinda hella dumb

SELECT 't3280', 'L1b' || topology.TopoGeo_AddLinestring('bug3280', geom)
FROM bug3280.edge where edge_id = 1
ORDER BY 1;

and if we do have an order by there, it should be ORDER BY 2 which actually works :)

SELECT 't3280', 'L1b' || topology.TopoGeo_AddLinestring('bug3280', geom)
FROM bug3280.edge where edge_id = 1
ORDER BY 21;

[robe]

In 16625:

Make test sort order deterministic by using lateral.
Closes #4111
(Note they are going to fix the issue upstream that made this fail on stable 10, but I wanted to change this test anyway)