Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack.

2024-03-08 Thread Jeff Davis
On Mon, 2024-03-04 at 18:10 +, Tom Lane wrote:
> Further further fix pg_upgrade crossversion test for adminpack.

Ever since this commit the cross-version upgrade test is failing (for
me, at least) with:

# Running: psql -X -v ON_ERROR_STOP=1 -c drop database if exists
contrib_regression_adminpack;
drop database if exists regression_adminpack -d port=53977
host=/tmp/EK6UT_TufI dbname='postgres'
ERROR:  DROP DATABASE cannot run inside a transaction block

It looks like when you added another command, the two were joined with
";\n", which ends up running the commands in a transaction block, which
doesn't work for DROP DATABASE.

I'm not sure how this test is succeeding for others, so perhaps I'm
doing something wrong?

Patch attached, though I'm not particularly great with perl and the
array flattening in my implementation might be too magical.

Regards,
Jeff Davis

From 54e13ddb01b859606ebe938a98587aef666df668 Mon Sep 17 00:00:00 2001
From: Jeff Davis 
Date: Fri, 8 Mar 2024 23:37:16 -0800
Subject: [PATCH] Fix cross-version pg_upgrade test.

---
 src/bin/pg_upgrade/t/002_pg_upgrade.pl | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index d951ed3af0..16b42d475f 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -246,15 +246,19 @@ if (defined($ENV{oldinstall}))
 
 	foreach my $updb (keys %$adjust_cmds)
 	{
-		my $upcmds = join(";\n", @{ $adjust_cmds->{$updb} });
+		my @command_args = ();
+		for my $upcmd (@{ $adjust_cmds->{$updb} })
+		{
+			push @command_args, '-c', $upcmd;
+		}
 
 		# For simplicity, use the newer version's psql to issue the commands.
 		$newnode->command_ok(
 			[
 'psql', '-X',
 '-v', 'ON_ERROR_STOP=1',
-'-c', $upcmds,
 '-d', $oldnode->connstr($updb),
+@command_args,
 			],
 			"ran version adaptation commands for database $updb");
 	}
-- 
2.34.1



pgsql: Document units of "timeout" in ConditionVariableTimedSleep()

2024-03-08 Thread Michael Paquier
Document units of "timeout" in ConditionVariableTimedSleep()

The timeout is passed down to WaitLatch() as milliseconds.

Author: Shveta Malik
Discussion: 
https://postgr.es/m/CAJpy0uC=xibqd1wapgyyvoiytap6uljaakld867zzxqu9ty...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/f160bf06f72ab242a8336eff108a1f4e69539b77

Modified Files
--
src/backend/storage/lmgr/condition_variable.c | 2 ++
1 file changed, 2 insertions(+)



Re: pgsql: Unicode case mapping tables and functions.

2024-03-08 Thread Jeff Davis
On Fri, 2024-03-08 at 10:24 +0200, Heikki Linnakangas wrote:
> On 07/03/2024 21:18, Jeff Davis wrote:
> > Unicode case mapping tables and functions.
> 
> With -Wtype-limits, I'm seeing this warning:

Thank you, fixed. Somehow I lost that flag from my script.

Can you please add some recommended compiler warning flags here:

https://wiki.postgresql.org/wiki/Committing_checklist

?

Regards,
Jeff Davis





pgsql: Fix type signedness error in commit 5c40364dd6.

2024-03-08 Thread Jeff Davis
Fix type signedness error in commit 5c40364dd6.

Use ssize_t instead of size_t.

Discussion: https://postgr.es/m/b20d6d97-7338-48ea-ba33-837a1c8ef...@iki.fi
Reported-by: Heikki Linnakangas

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/33ee2550d30bebb938238846369b2aae10e7f00f

Modified Files
--
src/common/unicode_case.c | 10 +-
src/include/common/unicode_case.h |  4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)



pgsql: Replace perror with custom postgres logging

2024-03-08 Thread Daniel Gustafsson
Replace perror with custom postgres logging

perror() is not used in postgres anymore out of policy, this replaces
the final callsites with the custom postgres logging framework.

Reviewed-by: Tom Lane 
Reviewed-by: Peter Eisentraut 
Discussion: https://postgr.es/m/89b00f63-40f7-4d82-8353-dc9cabbac...@yesql.se

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/6929e133b309d5d4568b5ed25b136935f63be618

Modified Files
--
src/common/exec.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)



pgsql: Fix errorhandling for reading from a pipe

2024-03-08 Thread Daniel Gustafsson
Fix errorhandling for reading from a pipe

When reading a line from a pipe failed on no data being read, the
errorhandling was erroneously logging with %m even thoug no error
description is available for %m to print.  This flaw accidentally
introduced in 5c7038d70bb.

Reported-by: Peter Eisentraut 
Discussion: 
https://postgr.es/m/baa34329-f431-46af-bf74-1a78fdc90...@eisentraut.org

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/be41a9b0380778a07386208afbf3f41ba7286cf3

Modified Files
--
src/common/exec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



pgsql: Improve WIN32 waiting logic in psql's \watch command.

2024-03-08 Thread Tom Lane
Improve WIN32 waiting logic in psql's \watch command.

do_watch had some leftover logic for enabling siglongjmp out of
waiting for input.  That's never done anything on Windows (cf.
psql_cancel_callback), and do_watch no longer relies on it for
non-Windows, so let's drop it.

Also, when the user cancels \watch by pressing ^C, the Windows
code would run the query one more time before exiting.  That doesn't
seem very desirable, and it's not what happens on other platforms.
Use the "done" flag similarly to non-Windows to avoid the extra query
execution.

Yugo Nagata (with minor fixes by me)

Discussion: 
https://postgr.es/m/20240305220552.85fd4afd6b6b8103bf4fe...@sraoss.co.jp

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/f07a20c8a3b15e71d3cbfcfed0600956042cbe74

Modified Files
--
src/bin/psql/command.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)



pgsql: Admit deferrable PKs into rd_pkindex, but flag them as such

2024-03-08 Thread Alvaro Herrera
Admit deferrable PKs into rd_pkindex, but flag them as such

... and in particular don't return them as replica identity.

The motivation for this change is letting the primary keys be seen by
code that derives NOT NULL constraints from them, when creating
inheritance children; before this change, if you had a deferrable PK,
pg_dump would not recreate the attnotnull marking properly, because the
column would not be considered as having anything to back said marking
after dropping the throwaway NOT NULL constraint.

The reason we don't want these PKs as replica identities is that
replication can corrupt data, if the uniqueness constraint is
transiently broken.

Reported-by: Amul Sul 
Reviewed-by: Dean Rasheed 
Discussion: 
https://postgr.es/m/caaj_b94qonkgsbdxofakhdnorqngafd1y3oa5qxfpqnjyxy...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/270af6f0df764d326e1a7355f4ce10dc73b05dac

Modified Files
--
src/backend/replication/logical/relation.c |  6 +-
src/backend/utils/cache/relcache.c | 27 ++--
src/include/utils/rel.h|  3 +-
src/test/regress/expected/constraints.out  | 92 ++
src/test/regress/expected/publication.out  | 10 +++
src/test/regress/expected/replica_identity.out |  5 ++
src/test/regress/sql/constraints.sql   | 25 +++
src/test/regress/sql/publication.sql   |  9 +++
src/test/regress/sql/replica_identity.sql  |  4 ++
9 files changed, 170 insertions(+), 11 deletions(-)



pgsql: Avoid stack overflow in ShowTransactionStateRec()

2024-03-08 Thread Alexander Korotkov
Avoid stack overflow in ShowTransactionStateRec()

The function recurses, but didn't perform stack-depth checks. It's
just a debugging aid, so instead of the usual check_stack_depth()
call, stop the printing if we'd risk stack overflow.

Here's an example of how to test this:

(n=100; printf "BEGIN;"; for ((i=1;i<=$n;i++)); do printf "SAVEPOINT 
s$i;"; done; printf "SET log_min_messages = 'DEBUG5'; SAVEPOINT sp;") | psql 
>/dev/null

In the passing, swap building the list of child XIDs and recursing to
parent. That saves memory while recursing, reducing the risk of out of
memory errors with lots of subtransactions. The saving is not very
significant in practice, but this order seems more logical anyway.

Report by Egor Chindyaskin and Alexander Lakhin.

Discussion: 
https://www.postgresql.org/message-id/1672760457.940462079%40f306.i.mail.ru
Author: Heikki Linnakangas
Reviewed-by: Robert Haas, Andres Freund, Alexander Korotkov

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/6f38c43eb135f5d6b873924180562d8334d6e31d

Modified Files
--
src/backend/access/transam/xact.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)



pgsql: Avoid recursion in MemoryContext functions

2024-03-08 Thread Alexander Korotkov
Avoid recursion in MemoryContext functions

You might run out of stack space with recursion, which is not nice in
functions that might be used e.g. at cleanup after transaction
abort. MemoryContext contains pointer to parent and siblings, so we
can traverse a tree of contexts iteratively, without using
stack. Refactor the functions to do that.

MemoryContextStats() still recurses, but it now has a limit to how
deep it recurses. Once the limit is reached, it prints just a summary
of the rest of the hierarchy, similar to how it summarizes contexts
with lots of children. That seems good anyway, because a context dump
with hundreds of nested contexts isn't very readable.

Report by Egor Chindyaskin and Alexander Lakhin.

Discussion: https://postgr.es/m/1672760457.940462079%40f306.i.mail.ru
Author: Heikki Linnakangas
Reviewed-by: Robert Haas, Andres Freund, Alexander Korotkov, Tom Lane

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/4c1973fcaecd9ef11de14ac55d3ec1432f6b82dc

Modified Files
--
src/backend/utils/mmgr/mcxt.c | 269 +-
src/include/utils/memutils.h  |   3 +-
2 files changed, 190 insertions(+), 82 deletions(-)



pgsql: Turn tail recursion into iteration in CommitTransactionCommand()

2024-03-08 Thread Alexander Korotkov
Turn tail recursion into iteration in CommitTransactionCommand()

Usually the compiler will optimize away the tail recursion anyway, but
if it doesn't, you can drive the function into stack overflow. For
example:

(n=100; printf "BEGIN;"; for ((i=1;i<=$n;i++)); do printf "SAVEPOINT 
s$i;"; done; printf "ERROR; COMMIT;") | psql >/dev/null

In order to get better readability and less changes to the existing code the
recursion-replacing loop is implemented as a wrapper function.

Report by Egor Chindyaskin and Alexander Lakhin.
Discussion: https://postgr.es/m/1672760457.940462079%40f306.i.mail.ru
Author: Alexander Korotkov, Heikki Linnakangas

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/fefd9a3fed275cecd9ed4091b00698deed39b92e

Modified Files
--
src/backend/access/transam/xact.c | 151 ++
1 file changed, 106 insertions(+), 45 deletions(-)



Re: pgsql: Unicode case mapping tables and functions.

2024-03-08 Thread Heikki Linnakangas

On 07/03/2024 21:18, Jeff Davis wrote:

Unicode case mapping tables and functions.


With -Wtype-limits, I'm seeing this warning:

unicode_case.c: In function ‘convert_case’:
unicode_case.c:107:47: warning: comparison of unsigned expression in ‘< 
0’ is always false [-Wtype-limits]
  107 | while (src[srcoff] != '\0' && (srclen < 0 || srcoff < 
srclen))

  |   ^

That seems like legit issue. The comment in unicode_strlower/upper() says:


 * String src must be encoded in UTF-8. If srclen < 0, src must be
 * NUL-terminated.


But srclen is of type size_t, which is unsigned.

--
Heikki Linnakangas
Neon (https://neon.tech)