Обсуждение: pgsql: Fix test case from b0c5b215d.

Fix test case from b0c5b215d.

I'd not checked that this iteration of the test actually worked
with a bootstrap superuser not named 'postgres'.  It didn't,
because the coercion rules for CASE caused us to try to cast
the 'postgres' literal to regrole.  Mea culpa.

Per buildfarm (via Alexander Korotkov)

Discussion: https://postgr.es/m/CAPpHfdsV=iTvH6B858hnH1bLgewYH6cdTnO_eOOw9EOa8kehkA@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/9d9ece4c16dbbaf3b9d60c2fe201b8e99a407be3

Modified Files
--------------
src/test/modules/test_pg_dump/expected/test_pg_dump.out | 8 ++++----
src/test/modules/test_pg_dump/sql/test_pg_dump.sql      | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)

On Tue, 30 Apr 2024 at 12:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fix test case from b0c5b215d.

Still failing for me and [1].

Maybe:

SELECT pg_describe_object(classid,objid,objsubid) COLLATE "C" AS obj,

Gets me the results in the expected order.

David

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2024-04-30%2000%3A27%3A04

David Rowley <dgrowleyml@gmail.com> writes:
> Maybe:
> SELECT pg_describe_object(classid,objid,objsubid) COLLATE "C" AS obj,

Argh, I knew better than to not specify COLLATE "C".
This commit's feeling a little snakebit.

            regards, tom lane

David Rowley <dgrowleyml@gmail.com> writes:
> Still failing for me and [1].
> Maybe:
> SELECT pg_describe_object(classid,objid,objsubid) COLLATE "C" AS obj,
> Gets me the results in the expected order.

I committed that suggestion, but I'm not sure it's enough to fix it,
because if I do

    LANG=cs_CZ.utf8 make check

then I still get variant output order.  I tried about six different
spellings of the query without improving matters, so I'm totally
baffled.  Have we managed to break COLLATE "C"?  (With one eye on
Jeff Davis' recent stuff, I could believe that, except I'd have
expected it to show up in other regression tests already.)

I'll await buildfarm results, but there's something odd here.

            regards, tom lane

I wrote:
> I committed that suggestion, but I'm not sure it's enough to fix it,
> because if I do
>     LANG=cs_CZ.utf8 make check
> then I still get variant output order.  I tried about six different
> spellings of the query without improving matters, so I'm totally
> baffled.  Have we managed to break COLLATE "C"?  (With one eye on
> Jeff Davis' recent stuff, I could believe that, except I'd have
> expected it to show up in other regression tests already.)

Yeah, the affected buildfarm members seem still not happy.

In simple testing such as
    select * from foo order by f1 collate "C";
the behavior seems as expected.  I also confirmed that I see the test
misbehavior locally with or without ICU.  So I kind of think that this
is not a failure in the sorting code per se.  My current idea is that
in a more complicated query such as the new test_pg_dump query, we are
somehow losing the COLLATE "C" specification --- it's there in the
Sort node according to EXPLAIN VERBOSE, but it sure doesn't seem to be
having any effect.

I'm too tired to look any closer tonight, though.

            regards, tom lane

On Tue, 30 Apr 2024 at 14:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah, the affected buildfarm members seem still not happy.

Isn't your latest fix adding COLLATE "C" to the wrong test?

Crake is failing with [1]:

--- /home/andrew/bf/root/HEAD/pgsql/src/test/modules/test_pg_dump/expected/test_pg_dump.out
2024-04-29 20:27:02.792272385 -0400
+++ /home/andrew/bf/root/HEAD/pgsql.build/testrun/test_pg_dump/regress/results/test_pg_dump.out
2024-04-29 20:31:43.662916513 -0400
@@ -145,8 +145,8 @@
 ----------------------------------------------------+-----------------------------+---------
  column c1 of foreign table ft1                     | role
regress_dump_test_role | a
  column c1 of table test_pg_dump_t1                 | role
regress_dump_test_role | a
- foreign table ft1                                  | role
regress_dump_test_role | a
  foreign-data wrapper dummy                         | role
regress_dump_test_role | a
+ foreign table ft1                                  | role
regress_dump_test_role | a
  function regress_pg_dump_schema.test_agg(smallint) | role
regress_dump_test_role | a
  function regress_pg_dump_schema.test_agg(smallint) | role
regress_dump_test_role | i
  function regress_pg_dump_schema.test_func()        | role
regress_dump_test_role | a

Whereas your fix in [2] adjusts some other tests at line 70 and 207
rather than around line 145.

The test at [3] should have the COLLATE "C".

David

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2024-04-30%2000%3A27%3A04
[2]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blobdiff;f=src/test/modules/test_pg_dump/expected/test_pg_dump.out;h=dd1a2389644ea7d8a5f8ae34a974525bde4cc817;hp=dc493e5be26e9ea97a07a2bd84fba48e979198ac;hb=900d1144256a63250a2e326567b636ee3220b731;hpb=9d9ece4c16dbbaf3b9d60c2fe201b8e99a407be3
[3]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/modules/test_pg_dump/expected/test_pg_dump.out;h=dd1a2389644ea7d8a5f8ae34a974525bde4cc817;hb=900d1144256a63250a2e326567b636ee3220b731#l138

David Rowley <dgrowleyml@gmail.com> writes:
> On Tue, 30 Apr 2024 at 14:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, the affected buildfarm members seem still not happy.

> Isn't your latest fix adding COLLATE "C" to the wrong test?

Oh!  No, it was definitely adjusting a test that needed it,
just not the only one :-(.  Thanks for the fresh eyes.

            regards, tom lane