Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alexander Lakhin <exclusion(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, buildfarm(at)sraoss(dot)co(dot)jp, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Christoph Berg <myon(at)debian(dot)org>, Michael Banck <mbanck(at)gmx(dot)net>, Tommy Pavlicek <tommypav122(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, jian(dot)universality(at)gmail(dot)com
Subject: Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)
Date: 2024-10-28 16:06:46
Message-ID: 3975515.1730131606@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alexander Lakhin <exclusion(at)gmail(dot)com> writes:
> 27.10.2024 20:41, Tom Lane wrote:
>>> In the no-good-deed-goes-unpunished department: buildfarm member
>>> hamerkop doesn't like this patch [1]. The diffs look like

> 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.)

Thank you for looking at this!

> I can really see different line endings with hexdump:
> 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 |

Wow. How are we producing \r\r\n? I'm not hugely surprised that some
versions of diff might see that as different from a single newline
even with --strip-trailing-cr --- formally, I think that's supposed
to make \r\n match \n, but the extra \r doesn't fit that pattern.

> 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.

Interesting. I believe we decided years ago that we didn't need to
use "rt" mode because that was the default on Windows, but was that
a misreading of the documentation? The link you provided doesn't
give any hint that there are more than two behaviors.

However ... the link you provided also mentions that text mode
includes treating control-Z as EOF; which I'd forgotten, but it
does make it less safe than I thought to use text mode for
reading script files.

What I'm now thinking is that we should revert 924e03917 after
all (that is, go back to using PG_BINARY_R) and instead make
read_whole_file manually squash \r\n to \n if we're on Windows.
Ugly, but I have yet to find anything about that platform that
isn't.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2024-10-28 16:09:13 Re: [PoC] Federated Authn/z with OAUTHBEARER
Previous Message Peter Eisentraut 2024-10-28 15:51:44 protocol-level wait-for-LSN