Re: pgsql: With gssencmode='require', check credential cache before connect

2024-04-08 Thread Heikki Linnakangas

On 09/04/2024 04:46, Kyotaro Horiguchi wrote:

Hello.

At Sun, 07 Apr 2024 23:50:08 +, Heikki Linnakangas 
 wrote in

With gssencmode='require', check credential cache before connecting


This commit adds the following error message (indentations are adjusted):

+   libpq_append_conn_error(conn,
+   "GSSAPI encryption required but it is not supported over a local 
socket)");

The closing parenthesis at the end of the message seems to be a
leftover from editing.


Fixed, thanks!


About the following message:

+   libpq_append_conn_error(conn, "could not set ssl alpn 
extension: %s", err);

I'm not sure about the policy for writing acronyms in lowercase, but
other occurrences of ALPN (in backend code) seem to be written in
uppercase.


Changed to uppercase. I also changed "ssl" to uppercase, for consistency 
with the "could not set SSL Server Name Indication (SNI)" message earlier.


To be even more consistent, we should perhaps spell out "SSL 
Application-Layer Protocol Negotiation (ALPN)", but that's pretty long 
and I don't think it really helps the user. It really should not fail, 
and there isn't anything the user can really do if that fails. Anyone 
who doesn't already know what ALPN is will need to google it anyway.


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





pgsql: libpq error message fixes

2024-04-08 Thread Heikki Linnakangas
libpq error message fixes

Remove stray paren, capitalize SSL and ALPN.

Author: Kyotaro Horiguchi
Discussion: 
https://www.postgresql.org/message-id/20240409.104613.1653854506705708036.horikyota@gmail.com

Branch
--
master

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

Modified Files
--
src/interfaces/libpq/fe-connect.c| 2 +-
src/interfaces/libpq/fe-secure-openssl.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)



pgsql: Fix typo in docs

2024-04-08 Thread Heikki Linnakangas
Fix typo in docs

Author: Erik Rijkers
Discussion: 
https://www.postgresql.org/message-id/0167b1e1-676c-66ba-e857-3ad7cd844...@xs4all.nl

Branch
--
master

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

Modified Files
--
doc/src/sgml/libpq.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



pgsql: Add missing set_pglocale_pgservice() for pg_walsummary and pg_co

2024-04-08 Thread Michael Paquier
Add missing set_pglocale_pgservice() for pg_walsummary and pg_combinebackup

These calls are required to make both tools work with NLS.

Author: Kyotaro Horiguchi
Discussion: 
https://postgr.es/m/20240408.162702.183779935636035593.horikyota@gmail.com

Branch
--
master

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

Modified Files
--
src/bin/pg_combinebackup/pg_combinebackup.c | 1 +
src/bin/pg_walsummary/pg_walsummary.c   | 1 +
2 files changed, 2 insertions(+)



Re: pgsql: Teach radix tree to embed values at runtime

2024-04-08 Thread John Naylor
On Tue, Apr 9, 2024 at 12:27 AM Andres Freund  wrote:
>
> This isn't quite C99 conformant, and thus breaks on the buildfarm animal
> set up to test that:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mylodon=2024-04-08%2012%3A07%3A01

I haven't forgotten about this and will fix within a few hours.




Re: pgsql: With gssencmode='require', check credential cache before connect

2024-04-08 Thread Kyotaro Horiguchi
Hello.

At Sun, 07 Apr 2024 23:50:08 +, Heikki Linnakangas 
 wrote in 
> With gssencmode='require', check credential cache before connecting

This commit adds the following error message (indentations are adjusted):

+   libpq_append_conn_error(conn,
+   "GSSAPI encryption required but it is not supported over a 
local socket)");

The closing parenthesis at the end of the message seems to be a
leftover from editing.

diff --git a/src/interfaces/libpq/fe-connect.c 
b/src/interfaces/libpq/fe-connect.c
index 4bd523ec6e..e35bdc4036 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2927,7 +2927,7 @@ keep_going:   
/* We will come back to here until there is
if (conn->raddr.addr.ss_family 
== AF_UNIX)
{

libpq_append_conn_error(conn,
-   
"GSSAPI encryption required but it is not supported 
over a local socket)");
+   
"GSSAPI encryption required but it is not supported 
over a local socket");
goto error_return;
}
if (conn->gcred == 
GSS_C_NO_CREDENTIAL)


About the following message:

+   libpq_append_conn_error(conn, "could not set ssl alpn 
extension: %s", err);

I'm not sure about the policy for writing acronyms in lowercase, but
other occurrences of ALPN (in backend code) seem to be written in
uppercase.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




pgsql: injection_points: Fix race condition with local injection point

2024-04-08 Thread Michael Paquier
injection_points: Fix race condition with local injection point tests

The module relies on a shmem exit callback to clean up any injection
points linked to a specific process.  One of the tests checks for the
case of an injection point name reused in a second connection where the
first connection should clean it up, but it did not count for the fact
that the shmem exit callback of the first connection may not have run
when the second connection begins its work.

The regress library includes a wait_pid() that can be used for this
purpose, instead of a custom wait logic, so let's rely on it to wait for
the first connection to exit before working with the second connection.
The module gains a REGRESS_OPTS to be able to look at the regress
library's dlpath.

This issue could be reproduced with a hardcoded sleep() in the shmem
exit callback, and the CI has been able to trigger it sporadically.

Oversight in f587338dec87.

Reported-by: Bharath Rupireddy
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/zhod3nxautteo...@paquier.xyz

Branch
--
master

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

Modified Files
--
src/test/modules/injection_points/Makefile  |  1 +
.../injection_points/expected/injection_points.out  | 17 +
src/test/modules/injection_points/meson.build   |  1 +
.../modules/injection_points/sql/injection_points.sql   | 16 
4 files changed, 35 insertions(+)



pgsql: In psql, avoid leaking a PGresult after a query is cancelled.

2024-04-08 Thread Tom Lane
In psql, avoid leaking a PGresult after a query is cancelled.

After a query cancel, the tail end of ExecQueryAndProcessResults
took care to clear any not-yet-read PGresults; but it forgot about
the one it has already read.  There would only be such a result
when handling a multi-command string made with "\;", so that you'd
have to cancel an earlier command in such a string to reach the
bug at all.  Even then, there would only be leakage of a single
PGresult per cancel, so it's not surprising nobody noticed this.
But a leak is a leak.

Noted while re-reviewing 90f517821, but this is independent of that:
it dates to 7844c9918.  Back-patch to v15 where that came in.

Branch
--
REL_16_STABLE

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

Modified Files
--
src/bin/psql/common.c | 2 ++
1 file changed, 2 insertions(+)



pgsql: In psql, avoid leaking a PGresult after a query is cancelled.

2024-04-08 Thread Tom Lane
In psql, avoid leaking a PGresult after a query is cancelled.

After a query cancel, the tail end of ExecQueryAndProcessResults
took care to clear any not-yet-read PGresults; but it forgot about
the one it has already read.  There would only be such a result
when handling a multi-command string made with "\;", so that you'd
have to cancel an earlier command in such a string to reach the
bug at all.  Even then, there would only be leakage of a single
PGresult per cancel, so it's not surprising nobody noticed this.
But a leak is a leak.

Noted while re-reviewing 90f517821, but this is independent of that:
it dates to 7844c9918.  Back-patch to v15 where that came in.

Branch
--
master

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

Modified Files
--
src/bin/psql/common.c | 2 ++
1 file changed, 2 insertions(+)



pgsql: In psql, avoid leaking a PGresult after a query is cancelled.

2024-04-08 Thread Tom Lane
In psql, avoid leaking a PGresult after a query is cancelled.

After a query cancel, the tail end of ExecQueryAndProcessResults
took care to clear any not-yet-read PGresults; but it forgot about
the one it has already read.  There would only be such a result
when handling a multi-command string made with "\;", so that you'd
have to cancel an earlier command in such a string to reach the
bug at all.  Even then, there would only be leakage of a single
PGresult per cancel, so it's not surprising nobody noticed this.
But a leak is a leak.

Noted while re-reviewing 90f517821, but this is independent of that:
it dates to 7844c9918.  Back-patch to v15 where that came in.

Branch
--
REL_15_STABLE

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

Modified Files
--
src/bin/psql/common.c | 2 ++
1 file changed, 2 insertions(+)



pgsql: Further review for re-implementation of psql's FETCH_COUNT featu

2024-04-08 Thread Tom Lane
Further review for re-implementation of psql's FETCH_COUNT feature.

Alexander Lakhin noted an obsolete comment, which led me to revisit
some other important comments in the patch, and that study turned up a
couple of unintended ways in which the chunked-fetch code path didn't
match the normal code path in ExecQueryAndProcessResults.  The only
nontrivial problem is that it didn't call PrintQueryStatus, so that
we'd not print the final status result from DML ... RETURNING
commands.  To avoid code duplication, move the filter for whether a
result is from RETURNING from PrintQueryResult to PrintQueryStatus.

Discussion: https://postgr.es/m/0023bea5-79c0-476e-96c8-dad599cc3...@gmail.com

Branch
--
master

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

Modified Files
--
src/bin/psql/common.c | 78 ---
1 file changed, 49 insertions(+), 29 deletions(-)



Re: pgsql: Teach radix tree to embed values at runtime

2024-04-08 Thread Andres Freund
Hi,

On 2024-04-08 11:57:01 +, John Naylor wrote:
> Teach radix tree to embed values at runtime
> 
> Previously, the decision to store values in leaves or within the child
> pointer was made at compile time, with variable length values using
> leaves by necessity. This commit allows introspecting the length of
> variable length values at runtime for that decision. This requires
> the ability to tell whether the last-level child pointer is actually
> a value, so we use a pointer tag in the lowest level bit.
> 
> Use this in TID store. This entails adding a byte to the header to
> reserve space for the tag. Commit f35bd9bf3 stores up to three offsets
> within the header with no bitmap, and now the header can be embedded
> as above. This reduces worst-case memory usage when TIDs are sparse.

This isn't quite C99 conformant, and thus breaks on the buildfarm animal
set up to test that:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mylodon=2024-04-08%2012%3A07%3A01

You can't have unnamed structs in C99, that's a C11 feature.  I wish we'd move
to C11, but ...

Greetings,

Andres




Re: pgsql: Transform OR clauses to ANY expression

2024-04-08 Thread Jelte Fennema-Nio
On Mon, 8 Apr 2024 at 17:10, Alexander Korotkov  wrote:
>
> On Mon, Apr 8, 2024 at 1:35 AM Melanie Plageman
>  wrote:
> > /src/backend/optimizer/prep/prepqual.c:582:33: warning: declaration of
> > ‘lc__state’ shadows a previous local [-Wshadow=compatible-local]
> >   582 | foreach(lc, entry->consts)
>
> Thank you for catching.  I'm fixing this now.

I noticed the fix in question, and I wanted to say that this whole
issue could've been avoided if the new foreach_ptr macros were used
(and thus arguably would have been a better way to fix this). Then
there wouldn't have been any ListCell shadowing, because no ListCell
would have been declared at all.




pgsql: Teach radix tree to embed values at runtime

2024-04-08 Thread John Naylor
Teach radix tree to embed values at runtime

Previously, the decision to store values in leaves or within the child
pointer was made at compile time, with variable length values using
leaves by necessity. This commit allows introspecting the length of
variable length values at runtime for that decision. This requires
the ability to tell whether the last-level child pointer is actually
a value, so we use a pointer tag in the lowest level bit.

Use this in TID store. This entails adding a byte to the header to
reserve space for the tag. Commit f35bd9bf3 stores up to three offsets
within the header with no bitmap, and now the header can be embedded
as above. This reduces worst-case memory usage when TIDs are sparse.

Reviewed (in an earlier version) by Masahiko Sawada

Discussion: 
https://postgr.es/m/CANWCAZYw+_KAaUNruhJfE=h6wgtbkedg32st8vbjbey82bg...@mail.gmail.com
Discussion: 
https://postgr.es/m/cad21aobci3hujzijubomo1tdwh3xtq9f89ctnq4bsqijomq...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/0fe5f64367bc2fc70baa1f0f993638830f8aa6a5

Modified Files
--
src/backend/access/common/tidstore.c | 81 +---
src/include/lib/radixtree.h  | 32 ++
2 files changed, 89 insertions(+), 24 deletions(-)



pgsql: Teach TID store to skip bitmap for small numbers of offsets

2024-04-08 Thread John Naylor
Teach TID store to skip bitmap for small numbers of offsets

The header portion of BlocktableEntry has enough padding space for
an array of 3 offsets (1 on 32-bit platforms). Use this space instead
of having a sparse bitmap array. This will take up a constant amount
of space no matter what the offsets are.

Reviewed (in an earlier version) by Masahiko Sawada

Discussion: 
https://postgr.es/m/CANWCAZYw+_KAaUNruhJfE=h6wgtbkedg32st8vbjbey82bg...@mail.gmail.com
Discussion: 
https://postgr.es/m/cad21aobci3hujzijubomo1tdwh3xtq9f89ctnq4bsqijomq...@mail.gmail.com

Branch
--
master

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

Modified Files
--
src/backend/access/common/tidstore.c   | 129 +++--
.../test_tidstore/expected/test_tidstore.out   |  28 +
.../modules/test_tidstore/sql/test_tidstore.sql|  10 ++
3 files changed, 130 insertions(+), 37 deletions(-)



pgsql: Provide a way block-level table AMs could re-use acquire_sample_

2024-04-08 Thread Alexander Korotkov
Provide a way block-level table AMs could re-use acquire_sample_rows()

While keeping API the same, this commit provides a way for block-level table
AMs to re-use existing acquire_sample_rows() by providing custom callbacks
for getting the next block and the next tuple.

Reported-by: Andres Freund
Discussion: 
https://postgr.es/m/20240407214001.jgpg5q3yv33ve6y3%40awork3.anarazel.de
Reviewed-by: Pavel Borisov

Branch
--
master

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

Modified Files
--
src/backend/access/heap/heapam_handler.c | 12 +++
src/backend/commands/analyze.c   | 42 +++-
src/include/access/tableam.h | 13 
src/include/commands/vacuum.h| 56 ++--
src/tools/pgindent/typedefs.list |  2 ++
5 files changed, 106 insertions(+), 19 deletions(-)



pgsql: Fix some grammer errors from error messages and codes comments

2024-04-08 Thread Alexander Korotkov
Fix some grammer errors from error messages and codes comments

Discussion: 
https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
Author: Tender Wang

Branch
--
master

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

Modified Files
--
src/backend/commands/tablecmds.c  | 18 +-
src/backend/parser/parse_utilcmd.c|  2 +-
src/backend/partitioning/partbounds.c | 10 +++---
src/include/nodes/parsenodes.h|  4 ++--
src/test/regress/expected/partition_split.out |  4 ++--
5 files changed, 21 insertions(+), 17 deletions(-)



pgsql: Fill CommonRdOptions with default values in extract_autovac_opts

2024-04-08 Thread Alexander Korotkov
Fill CommonRdOptions with default values in extract_autovac_opts()

Reported-by: Thomas Munro
Reported-by: Pavel Borisov
Discussion: 
https://postgr.es/m/CA%2BhUKGLZzLR50RBvuqOO3MZ%3DF54ETz-rTp1PDX9uDGP_GqyYqA%40mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/422041542f313f23ca66cad26e9b2b99c4d1999a

Modified Files
--
src/backend/access/common/reloptions.c | 28 
src/backend/postmaster/autovacuum.c|  1 +
src/backend/utils/cache/relcache.c | 21 +
src/include/access/reloptions.h|  1 +
4 files changed, 31 insertions(+), 20 deletions(-)



Re: pgsql: Enhance libpq encryption negotiation tests with new GUC

2024-04-08 Thread Heikki Linnakangas

On 08/04/2024 09:40, Kyotaro Horiguchi wrote:

At Sun, 07 Apr 2024 23:50:08 +, Heikki Linnakangas 
 wrote in

Enhance libpq encryption negotiation tests with new GUC


This commit adds the following messages:


gettext_noop("Log details of pre-authentication connection handshake."),


Similar to a nearby commit, other messages with a similar context use
the phrase "Logs ". Wouldn't it be better to align this
message with existing ones?

diff --git a/src/backend/utils/misc/guc_tables.c 
b/src/backend/utils/misc/guc_tables.c
index 83e3a59d7e..4584829992 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1227,7 +1227,7 @@ struct config_bool ConfigureNamesBool[] =
},
{
{"trace_connection_negotiation", PGC_POSTMASTER, 
DEVELOPER_OPTIONS,
-   gettext_noop("Log details of pre-authentication connection 
handshake."),
+   gettext_noop("Logs details of pre-authentication connection 
handshake."),
NULL,
GUC_NOT_IN_SAMPLE
},


We're not very consistent about it, there's also:

log_temp_files: Log the use of temporary files larger than this number 
of kilobytes.

trace_syncscan: Generate debugging output for synchronized scanning.
trace_sort: Emit information about resource usage in sorting.
backtrace_functions: Log backtrace for errors in these functions.
backtrace_on_internal_error: Log backtrace for any error with error code 
XX000 (internal error).


But I agree the "Logs ..." phrasing is more common, so committed.

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





pgsql: Adjust wording of trace_connection_negotiation GUC's description

2024-04-08 Thread Heikki Linnakangas
Adjust wording of trace_connection_negotiation GUC's description

We're not very consistent about this across all the GUCs, but the
"Logs ..." phrasing is more common than "Log ...", and is used by the
neighboring "log_connections" and "log_disconnections" GUCs, so switch
to that.

Author: Kyotaro Horiguchi
Discussion: 
https://www.postgresql.org/message-id/20240408.154010.1170771365226258348.horikyota@gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/3dbd2ff78654b2ac484a8d08ace788c492117894

Modified Files
--
src/backend/utils/misc/guc_tables.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Re: pgsql: Custom reloptions for table AM

2024-04-08 Thread Alexander Korotkov
Hi, Thomas!

On Mon, Apr 8, 2024 at 12:03 PM Thomas Munro  wrote:
> I think this is uninitialised memory:

I'm already working on this.  Will be fixed in 10-15 minutes.

--
Regards,
Alexander Korotkov




Re: pgsql: Custom reloptions for table AM

2024-04-08 Thread Thomas Munro
Hi Alexander,

I think this is uninitialised memory:

../pgsql/src/backend/postmaster/autovacuum.c:2988:33: runtime error:
load of value 80, which is not a valid value for type '_Bool'
#0 0x56010b3b6e47 in relation_needs_vacanalyze
../pgsql/src/backend/postmaster/autovacuum.c:2988
#1 0x56010b3b7745 in do_autovacuum
../pgsql/src/backend/postmaster/autovacuum.c:2023
#2 0x56010b3ba1a3 in AutoVacWorkerMain
../pgsql/src/backend/postmaster/autovacuum.c:1569
#3 0x56010b3c0aa6 in postmaster_child_launch
../pgsql/src/backend/postmaster/launch_backend.c:265
#4 0x56010b3c36b4 in StartChildProcess
../pgsql/src/backend/postmaster/postmaster.c:3928
#5 0x56010b3c6775 in StartAutovacuumWorker
../pgsql/src/backend/postmaster/postmaster.c:3997
#6 0x56010b3c6775 in process_pm_pmsignal
../pgsql/src/backend/postmaster/postmaster.c:3809
#7 0x56010b3c6775 in ServerLoop
../pgsql/src/backend/postmaster/postmaster.c:1667
#8 0x56010b3c8ca5 in PostmasterMain
../pgsql/src/backend/postmaster/postmaster.c:1372
#9 0x56010b1f458e in main ../pgsql/src/backend/main/main.c:197
#10 0x7f23b7e456c9 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#11 0x7f23b7e45784 in __libc_start_main_impl ../csu/libc-start.c:360
#12 0x56010ad5a270 in _start
(/home/bf/bf-build/tamandua/HEAD/pgsql.build/tmp_install/home/bf/bf-build/tamandua/HEAD/inst/bin/postgres+0x8b3270)
(BuildId: 17e96a9e15a2c22c2062d2cefc7586aaea64b144)

Or from CI:

../src/backend/postmaster/autovacuum.c:2988:33: runtime error: load of
value 26, which is not a valid value for type '_Bool'
==43234==Using libbacktrace symbolizer.
#0 0x556ee56de975 in relation_needs_vacanalyze
../src/backend/postmaster/autovacuum.c:2988
#1 0x556ee56df9e0 in do_autovacuum
../src/backend/postmaster/autovacuum.c:2023
#2 0x556ee56e1ada in AutoVacWorkerMain
../src/backend/postmaster/autovacuum.c:1569
#3 0x556ee56e8175 in postmaster_child_launch
../src/backend/postmaster/launch_backend.c:265
#4 0x556ee56ea1f3 in StartChildProcess
../src/backend/postmaster/postmaster.c:3928
#5 0x556ee56ede6f in StartAutovacuumWorker
../src/backend/postmaster/postmaster.c:3997
#6 0x556ee56ee3b0 in process_pm_pmsignal
../src/backend/postmaster/postmaster.c:3809
#7 0x556ee56ee889 in ServerLoop ../src/backend/postmaster/postmaster.c:1667
#8 0x556ee56f0478 in PostmasterMain
../src/backend/postmaster/postmaster.c:1372
#9 0x556ee550fb70 in main ../src/backend/main/main.c:197
#10 0x7f81c899cd09 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
#11 0x556ee505f249 in _start
(/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres+0x8ea249)




pgsql: Custom reloptions for table AM

2024-04-08 Thread Alexander Korotkov
Custom reloptions for table AM

Let table AM define custom reloptions for its tables. This allows specifying
AM-specific parameters by the WITH clause when creating a table.

The reloptions, which could be used outside of table AM, are now extracted
into the CommonRdOptions data structure.  These options could be by decision
of table AM directly specified by a user or calculated in some way.

The new test module test_tam_options evaluates the ability to set up custom
reloptions and calculate fields of CommonRdOptions on their base.

The code may use some parts from prior work by Hao Wu.

Discussion: 
https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Discussion: 
https://postgr.es/m/AMUA1wBBBxfc3tKRLLdU64rb.1.1683276279979.Hmail.wuhao%40hashdata.cn
Reviewed-by: Reviewed-by: Pavel Borisov, Matthias van de Meent, Jess Davis

Branch
--
master

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

Modified Files
--
src/backend/access/common/reloptions.c | 121 -
src/backend/access/heap/heapam.c   |   4 +-
src/backend/access/heap/heapam_handler.c   |  13 ++
src/backend/access/heap/heaptoast.c|   9 +-
src/backend/access/heap/hio.c  |   4 +-
src/backend/access/heap/pruneheap.c|   4 +-
src/backend/access/heap/rewriteheap.c  |   4 +-
src/backend/access/table/tableam.c |   2 +-
src/backend/access/table/tableamapi.c  |  25 
src/backend/commands/createas.c|  13 +-
src/backend/commands/tablecmds.c   |  63 +
src/backend/commands/vacuum.c  |   8 +-
src/backend/postmaster/autovacuum.c|  12 +-
src/backend/tcop/utility.c |  28 +++-
src/backend/utils/cache/relcache.c |  73 +-
src/include/access/reloptions.h|  10 +-
src/include/access/tableam.h   |  50 +++
src/include/utils/rel.h| 148 +++--
src/test/modules/Makefile  |   1 +
src/test/modules/meson.build   |   1 +
src/test/modules/test_tam_options/.gitignore   |   4 +
src/test/modules/test_tam_options/Makefile |  23 
.../test_tam_options/expected/test_tam_options.out |  36 +
src/test/modules/test_tam_options/meson.build  |  33 +
.../test_tam_options/sql/test_tam_options.sql  |  25 
.../test_tam_options/test_tam_options--1.0.sql |  12 ++
.../modules/test_tam_options/test_tam_options.c|  66 +
.../test_tam_options/test_tam_options.control  |   4 +
src/tools/pgindent/typedefs.list   |   4 +-
29 files changed, 639 insertions(+), 161 deletions(-)



pgsql: Fix the intermittent buildfarm failures in 040_standby_failover_

2024-04-08 Thread Amit Kapila
Fix the intermittent buildfarm failures in 040_standby_failover_slots_sync.

It is possible that even if the primary waits for the subscriber to catch
up and then disables the subscription, the XLOG_RUNNING_XACTS record gets
inserted between the two steps by bgwriter and walsender processes it.
This can move the restart_lsn of the corresponding slot in an
unpredictable way which further leads to slot sync failure.

To ensure predictable behaviour, we drop the subscription and manually
create the slot before the test. The other idea we discussed to write a
predictable test is to use injection points to control the bgwriter
logging XLOG_RUNNING_XACTS but that needs more analysis. We can add a
separate test using injection points.

Per buildfarm

Author: Hou Zhijie
Reviewed-by: Amit Kapila, Shveta Malik
Discussion: 
https://postgr.es/m/CAA4eK1JD8h_XLRsK_o_Xh=5MhTzm+6d4Cb4_uPgFJ2wSQDah=g...@mail.gmail.com

Branch
--
master

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

Modified Files
--
.../recovery/t/040_standby_failover_slots_sync.pl  | 62 +-
1 file changed, 24 insertions(+), 38 deletions(-)



pgsql: Use bump context for TID bitmaps stored by vacuum

2024-04-08 Thread John Naylor
Use bump context for TID bitmaps stored by vacuum

Vacuum does not pfree individual entries, and only frees the entire
storage space when finished with it. This allows using a bump context,
eliminating the chunk header in each leaf allocation. Most leaf
allocations will be 16 to 32 bytes, so that's a significant savings.
TidStoreCreateLocal gets a boolean parameter to indicate that the
created store is insert-only.

This requires a separate tree context for iteration, since we free
the iteration state after iteration completes.

Discussion: 
https://postgr.es/m/CANWCAZac%3DpBePg3rhX8nXkUuaLoiAJJLtmnCfZsPEAS4EtJ%3Dkg%40mail.gmail.com
Discussion: 
https://postgr.es/m/canwcazzqffxvzo8yzhfwtqv+z2gamv1ku16vu7kwmb5kzqy...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/8a1b31e6e59631807a08a4e9465134c343bbdf5e

Modified Files
--
src/backend/access/common/tidstore.c   | 15 +--
src/backend/access/heap/vacuumlazy.c   |  4 ++--
src/include/access/tidstore.h  |  2 +-
src/include/lib/radixtree.h| 11 ++-
src/test/modules/test_tidstore/test_tidstore.c |  3 ++-
5 files changed, 28 insertions(+), 7 deletions(-)



Re: pgsql: Transform OR clauses to ANY expression

2024-04-08 Thread Alexander Korotkov
On Mon, Apr 8, 2024 at 9:24 AM Kyotaro Horiguchi
 wrote:
> At Mon, 08 Apr 2024 14:46:57 +0900 (JST), Kyotaro Horiguchi 
>  wrote in
> > At Sun, 07 Apr 2024 22:28:06 +, Alexander Korotkov 
> >  wrote in
> > > Transform OR clauses to ANY expression
> >
> > This commit introduces a message like this:
> >
> > > gettext_noop("Set the minimum length of the list of OR clauses to attempt 
> > > the OR-to-ANY transformation."),
> >
> > Unlike the usual phrasing of similar messages in this context, which
> > use the form "Sets the minimum length of...", this message uses an
> > imperative form ("Set").  I believe it would be better to align the
> > tone of this message with the others by changing "Set" to "Sets".
> >
> > regards.
> >
> >
> > diff --git a/src/backend/utils/misc/guc_tables.c 
> > b/src/backend/utils/misc/guc_tables.c
> > index 83e3a59d7e..a527ffe0b0 100644
> > --- a/src/backend/utils/misc/guc_tables.c
> > +++ b/src/backend/utils/misc/guc_tables.c
> > @@ -3670,7 +3670,7 @@ struct config_int ConfigureNamesInt[] =
> >
> >   {
> >   {"or_to_any_transform_limit", PGC_USERSET, QUERY_TUNING_OTHER,
> > - gettext_noop("Set the minimum length of the list of 
> > OR clauses to attempt the OR-to-ANY transformation."),
> > + gettext_noop("Sets the minimum length of the list of 
> > OR clauses to attempt the OR-to-ANY transformation."),
> >   gettext_noop("Once the limit is reached, the planner 
> > will try to replace expression like "
> >"'x=c1 OR x=c2 ..' to the 
> > expression 'x = ANY(ARRAY[c1,c2,..])'"),
> >   GUC_EXPLAIN
>
> Sorry for the sequential posts, but I found the following contruct in
> the same patch to be quite untranslatable.

No worries.  But these are distinct patches.

> > errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
> >   first ? "lower" : "upper",
> >   relname,
> >   defaultPart ? (first ? "less than" : "greater than") : "not 
> > equals to",
> >   first ? "lower" : "upper"),
> >   parser_errposition(pstate, datum->location)));
>
> I might be misunderstanding this, but if the expressions are correct,
> the message could be divided into four fixed messages based on "first"
> and "defaultPart".  Alternatively, it might be better to provide
> simpler explation of the situation.

Yes, splitting this into multiple messages helps both translating and
readability.  Will be fixed in the next couple of days.

--
Regards,
Alexander Korotkov




pgsql: Fix JsonExpr deparsing to emit QUOTES and WRAPPER correctly

2024-04-08 Thread Amit Langote
Fix JsonExpr deparsing to emit QUOTES and WRAPPER correctly

Currently, get_json_expr_options() does not emit the default values
for QUOTES (KEEP QUOTES) and WRAPPER (WITHOUT WRAPPER).  That causes
the deparsed JSON_TABLE() columns, such as those contained in a a
view's query, to behave differently when executed than the original
definition.  That's because the rules encoded in
transformJsonTableColumns() will choose either JSON_VALUE() or
JSON_QUERY() as implementation to execute a given column's path
expression depending on the QUOTES and WRAPPER specificationd and
they have slightly different semantics.

Reported-by: Jian He 
Discussion: 
https://postgr.es/m/CACJufxEqhqsfrg_p7EMyo5zak3d767iFDL8vz_4%3DZBHpOtrghw%40mail.gmail.com

Branch
--
master

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

Modified Files
--
src/backend/utils/adt/ruleutils.c|  6 
src/test/regress/expected/sqljson_jsontable.out  | 44 
src/test/regress/expected/sqljson_queryfuncs.out | 26 +++---
3 files changed, 41 insertions(+), 35 deletions(-)



pgsql: Fix restriction on specifying KEEP QUOTES in JSON_QUERY()

2024-04-08 Thread Amit Langote
Fix restriction on specifying KEEP QUOTES in JSON_QUERY()

Currently, transformJsonFuncExpr() enforces some restrictions on
the combinations of QUOTES and WRAPPER clauses that can be specified
in JSON_QUERY().  The intent was to only prevent the useless
combination WITH WRAPPER OMIT QUOTES, but the coding prevented KEEP
QUOTES too, which is not helpful. Fix that.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/561b74ddb8781f8c0511f6473c51fb51c8c6b087

Modified Files
--
src/backend/parser/parse_expr.c  |  2 +-
src/test/regress/expected/sqljson_jsontable.out  | 13 +
src/test/regress/expected/sqljson_queryfuncs.out | 36 +++-
src/test/regress/sql/sqljson_jsontable.sql   |  5 ++--
src/test/regress/sql/sqljson_queryfuncs.sql  | 11 
5 files changed, 41 insertions(+), 26 deletions(-)



pgsql: JSON_TABLE: Add support for NESTED paths and columns

2024-04-08 Thread Amit Langote
JSON_TABLE: Add support for NESTED paths and columns

A NESTED path allows to extract data from nested levels of JSON
objects given by the parent path expression, which are projected as
columns specified using a nested COLUMNS clause, just like the parent
COLUMNS clause.  Rows comprised from a NESTED columns are "joined"
to the row comprised from the parent columns.  If a particular NESTED
path evaluates to 0 rows, then the nested COLUMNS will emit NULLs,
making it an OUTER join.

NESTED columns themselves may include NESTED paths to allow
extracting data from arbitrary nesting levels, which are likewise
joined against the rows at the parent level.

Multiple NESTED paths at a given level are called "sibling" paths
and their rows are combined by UNIONing them, that is, after being
joined against the parent row as described above.

Author: Nikita Glukhov 
Author: Teodor Sigaev 
Author: Oleg Bartunov 
Author: Alexander Korotkov 
Author: Andrew Dunstan 
Author: Amit Langote 
Author: Jian He 

Reviewers have included (in no particular order):

Andres Freund, Alexander Korotkov, Pavel Stehule, Andrew Alsup,
Erik Rijkers, Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson,
Justin Pryzby, Álvaro Herrera, Jian He

Discussion: 
https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886dea...@postgrespro.ru
Discussion: 
https://postgr.es/m/20220616233130.rparivafipt6d...@alap3.anarazel.de
Discussion: 
https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org
Discussion: 
https://postgr.es/m/CA+HiwqE4XTdfb1nW=ojoy_tqsrhyt-q_kb6i5d4xckyrlc1...@mail.gmail.com

Branch
--
master

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

Modified Files
--
doc/src/sgml/func.sgml | 154 -
src/backend/catalog/sql_features.txt   |   2 +-
src/backend/nodes/nodeFuncs.c  |   2 +
src/backend/parser/gram.y  |  38 +-
src/backend/parser/parse_jsontable.c   | 150 +++-
src/backend/utils/adt/jsonpath_exec.c  | 168 -
src/backend/utils/adt/ruleutils.c  |  60 +++-
src/include/nodes/parsenodes.h |   2 +
src/include/nodes/primnodes.h  |  34 +-
src/include/parser/kwlist.h|   1 +
.../ecpg/test/expected/sql-sqljson_jsontable.c |  14 +-
.../test/expected/sql-sqljson_jsontable.stderr |   8 +
.../test/expected/sql-sqljson_jsontable.stdout |   1 +
src/interfaces/ecpg/test/sql/sqljson_jsontable.pgc |   8 +
src/test/regress/expected/sqljson_jsontable.out| 385 +
src/test/regress/sql/sqljson_jsontable.sql | 210 +++
src/tools/pgindent/typedefs.list   |   1 +
17 files changed, 1207 insertions(+), 31 deletions(-)



Re: pgsql: Enhance libpq encryption negotiation tests with new GUC

2024-04-08 Thread Kyotaro Horiguchi
At Sun, 07 Apr 2024 23:50:08 +, Heikki Linnakangas 
 wrote in 
> Enhance libpq encryption negotiation tests with new GUC

This commit adds the following messages:

> gettext_noop("Log details of pre-authentication connection handshake."),

Similar to a nearby commit, other messages with a similar context use
the phrase "Logs ". Wouldn't it be better to align this
message with existing ones?

diff --git a/src/backend/utils/misc/guc_tables.c 
b/src/backend/utils/misc/guc_tables.c
index 83e3a59d7e..4584829992 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1227,7 +1227,7 @@ struct config_bool ConfigureNamesBool[] =
},
{
{"trace_connection_negotiation", PGC_POSTMASTER, 
DEVELOPER_OPTIONS,
-   gettext_noop("Log details of pre-authentication 
connection handshake."),
+   gettext_noop("Logs details of pre-authentication 
connection handshake."),
NULL,
GUC_NOT_IN_SAMPLE
},

regards

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




Re: pgsql: Transform OR clauses to ANY expression

2024-04-08 Thread Kyotaro Horiguchi
At Mon, 08 Apr 2024 14:46:57 +0900 (JST), Kyotaro Horiguchi 
 wrote in 
> At Sun, 07 Apr 2024 22:28:06 +, Alexander Korotkov 
>  wrote in 
> > Transform OR clauses to ANY expression
> 
> This commit introduces a message like this:
> 
> > gettext_noop("Set the minimum length of the list of OR clauses to attempt 
> > the OR-to-ANY transformation."),
> 
> Unlike the usual phrasing of similar messages in this context, which
> use the form "Sets the minimum length of...", this message uses an
> imperative form ("Set").  I believe it would be better to align the
> tone of this message with the others by changing "Set" to "Sets".
> 
> regards.
> 
> 
> diff --git a/src/backend/utils/misc/guc_tables.c 
> b/src/backend/utils/misc/guc_tables.c
> index 83e3a59d7e..a527ffe0b0 100644
> --- a/src/backend/utils/misc/guc_tables.c
> +++ b/src/backend/utils/misc/guc_tables.c
> @@ -3670,7 +3670,7 @@ struct config_int ConfigureNamesInt[] =
>  
>   {
>   {"or_to_any_transform_limit", PGC_USERSET, QUERY_TUNING_OTHER,
> - gettext_noop("Set the minimum length of the list of OR 
> clauses to attempt the OR-to-ANY transformation."),
> + gettext_noop("Sets the minimum length of the list of OR 
> clauses to attempt the OR-to-ANY transformation."),
>   gettext_noop("Once the limit is reached, the planner 
> will try to replace expression like "
>"'x=c1 OR x=c2 ..' to the 
> expression 'x = ANY(ARRAY[c1,c2,..])'"),
>   GUC_EXPLAIN

Sorry for the sequential posts, but I found the following contruct in
the same patch to be quite untranslatable.

> errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
>   first ? "lower" : "upper",
>   relname,
>   defaultPart ? (first ? "less than" : "greater than") : "not 
> equals to",
>   first ? "lower" : "upper"),
>   parser_errposition(pstate, datum->location)));

I might be misunderstanding this, but if the expressions are correct,
the message could be divided into four fixed messages based on "first"
and "defaultPart".  Alternatively, it might be better to provide
simpler explation of the situation.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




pgsql: Fix the wording of or_to_any_transform_limit description

2024-04-08 Thread Alexander Korotkov
Fix the wording of or_to_any_transform_limit description

Reported-by: Kyotaro Horiguchi
Discussion: 
https://postgr.es/m/20240408.144657.1746688590065601661.horikyota.ntt%40gmail.com

Branch
--
master

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

Modified Files
--
src/backend/utils/misc/guc_tables.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



pgsql: Fix the value of or_to_any_transform_limit in postgresql.conf.sa

2024-04-08 Thread Alexander Korotkov
Fix the value of or_to_any_transform_limit in postgresql.conf.sample

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/ZhM8jH8gsKm5Q-9p%40pryzbyj2023

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/2af75e1174786a02fc35755c0cb98c522d72a065

Modified Files
--
src/backend/utils/misc/postgresql.conf.sample | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)