Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR) - Mailing list pgsql-hackers
From | Alexander Lakhin |
---|---|
Subject | Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR) |
Date | |
Msg-id | 79284195-4993-7b00-f6df-8db28ca60fa3@gmail.com Whole thread Raw |
In response to | Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR) (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)
|
List | pgsql-hackers |
Hello Tom, 27.10.2024 20:41, Tom Lane wrote: > I wrote: >> In the no-good-deed-goes-unpunished department: buildfarm member >> hamerkop doesn't like this patch [1]. The diffs look like >> ... >> So what I'd like to do to fix this is to change >> - if ((file = AllocateFile(filename, PG_BINARY_R)) == NULL) >> + if ((file = AllocateFile(filename, "r")) == NULL) > Well, that didn't fix it :-(. I went so far as to extract the raw log > files from the buildfarm database, and what they show is that there is > absolutely no difference between the lines diff is claiming are > different: > > -QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL\r\n > +QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL\r\n > > It's the same both before and after 924e03917, which made the code > change depicted above, so that didn't help. > > So I'm pretty baffled. I suppose the expected and result files must > actually be different, and something in subsequent processing is > losing the difference before it gets to the buildfarm database. > But I don't have the ability to debug that from here. Does anyone > with access to hamerkop want to poke into this? > > Without additional information, the only thing I can think of that > I have any confidence will eliminate these failures is to reformat > the affected test cases so that they produce just a single line of > output. That's kind of annoying from a functionality-coverage point > of view, but I'm not sure avoiding it is worth moving mountains for. > I've managed to reproduce the issue with the core.autocrlf=true git setting (which sets CR+LF line ending in test_ext7--2.0--2.1bad.sql) and with diff from msys: C:\msys64\usr\bin\diff.exe --version diff (GNU diffutils) 3.8 (Gnu/Win32 Diff [1] doesn't detect those EOL differences and thus the test doesn't fail.) I can really see different line endings with hexdump: hexdump -C ...testrun\test_extensions\regress\regression.diffs 00000230 20 20 20 20 20 20 20 20 5e 0a 2d 51 55 45 52 59 | ^.-QUERY| 00000240 3a 20 20 43 52 45 41 54 45 20 46 55 4e 43 54 49 |: CREATE FUNCTI| 00000250 4e 20 6d 79 5f 65 72 72 6f 6e 65 6f 75 73 5f 66 |N my_erroneous_f| 00000260 75 6e 63 28 69 6e 74 29 20 52 45 54 55 52 4e 53 |unc(int) RETURNS| 00000270 20 69 6e 74 20 4c 41 4e 47 55 41 47 45 20 53 51 | int LANGUAGE SQ| 00000280 4c 0a 2b 51 55 45 52 59 3a 20 20 43 52 45 41 54 |L.+QUERY: CREAT| 00000290 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 72 |E FUNCTIN my_err| 000002a0 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 29 |oneous_func(int)| 000002b0 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 4e | RETURNS int LAN| 000002c0 47 55 41 47 45 20 53 51 4c 0d 0a 20 41 53 20 24 |GUAGE SQL.. AS $| hexdump -C .../testrun/test_extensions/regress/results/test_extensions.out | grep -C5 FUNCTIN 00000b80 20 5e 0d 0a 51 55 45 52 59 3a 20 20 43 52 45 41 | ^..QUERY: CREA| 00000b90 54 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 |TE FUNCTIN my_er| 00000ba0 72 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 |roneous_func(int| 00000bb0 29 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 |) RETURNS int LA| 00000bc0 4e 47 55 41 47 45 20 53 51 4c 0d 0d 0a 41 53 20 |NGUAGE SQL...AS | whilst hexdump -C .../src/test/modules/test_extensions/expected/test_extensions.out | grep -C5 FUNCTIN 00000b80 20 5e 0d 0a 51 55 45 52 59 3a 20 20 43 52 45 41 | ^..QUERY: CREA| 00000b90 54 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 |TE FUNCTIN my_er| 00000ba0 72 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 |roneous_func(int| 00000bb0 29 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 |) RETURNS int LA| 00000bc0 4e 47 55 41 47 45 20 53 51 4c 0d 0a 41 53 20 24 |NGUAGE SQL..AS $| It looks like --strip-trailing-cr doesn't work as desired for this diff version. I've also dumped buf in read_whole_file() and found that in both PG_BINARY_R and "r" modes the 0d 0a ending is preserved. But it changed to 0a with the "rt" mode (see [1]), and it makes the test (and the whole `meson test`) pass for me. [1] https://gnuwin32.sourceforge.net/packages/diffutils.htm [2] https://learn.microsoft.com/en-us/cpp/c-runtime-library/file-translation-constants?view=msvc-170 Best regards, Alexander
pgsql-hackers by date: