Re: pgsql: Refactor libpq state machine for negotiating encryption

2024-04-12 Thread Heikki Linnakangas

On 11/04/2024 20:07, Heikki Linnakangas wrote:

On 11/04/2024 02:33, Thomas Munro wrote:

On Thu, Apr 11, 2024 at 11:25 AM Tom Lane  wrote:

Thomas Munro  writes:

If -Dssl=none and -Dgssapi=disabled, compilation of fe-connect.c
fails: call to undeclared function 'encryption_negotiation_failed'.  I
didn't look too hard, but maybe ENABLE_GSS and USE_GSS are confused?


For me, configure --with-gssapi fails like that, but the other three
combinations of --with-openssl and --with-gssapi compile OK.  I don't
find it terribly surprising that the buildfarm isn't covering that
combination ...


Oops, right, correction to my report: it is indeed -Dssl=none
-Dgssapi=enabled that is broken, not the other combinations.


Yes, I misspelled ENABLE_GSS as USE_GSS.

After fixing that, the new tests are failing; the expected output for
many of the cases is different when GSSAPI support is not compiled in. I
think the test tables need to be rearranged some more to take that into
account, or we will end up with a ridiculous amount of different
expected outputs.

I will take a closer look at that tomorrow. As a bandaid fix, we could
temporarily disable the new tests with that combination of configure
options, it's still better test coverage than not having the tests at
all. But given that no buildfarm members are testing that combination I
think it can wait a day.


Fixed the compilation with that combination, and the expected test 
output. Thanks for the report!


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





pgsql: Document PG_TEST_EXTRA=libpq_encryption and also check 'kerberos

2024-04-12 Thread Heikki Linnakangas
Document PG_TEST_EXTRA=libpq_encryption and also check 'kerberos'

In the libpq encryption negotiation tests, don't run the GSSAPI tests
unless PG_TEST_EXTRA='kerberos' is also set. That makes it possible to
still run most of the tests when GSSAPI support is compiled in, but
there's no MIT Kerberos installation.

Branch
--
master

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

Modified Files
--
doc/src/sgml/regress.sgml  | 14 +-
src/interfaces/libpq/t/005_negotiate_encryption.pl | 14 ++
2 files changed, 23 insertions(+), 5 deletions(-)



pgsql: Fix libpq_encryption tests when compiled without SSL support

2024-04-12 Thread Heikki Linnakangas
Fix libpq_encryption tests when compiled without SSL support

It correctly skipped tests involving SSL in the server when SSL
support was not compiled in, but even when SSL is not enabled in the
server and the connection is established without SSL, libpq behaves
differently in many of the test scenarios when libpq is compiled
without SSL support. For example, with sslmode=prefer, if libpq is
compiled with SSL support it will attempt to use SSL, but without SSL
support it will try authenticating in plaintext mode directly. The
expected test output didn't take that into account.

Discussion: 
https://www.postgresql.org/message-id/ca%2bhukg%2bhrttb%2bx%2bkkkj_cfx6snhbeguqmgxjgmwdvpg7ygf...@mail.gmail.com

Branch
--
master

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

Modified Files
--
.../libpq_encryption/t/001_negotiate_encryption.pl | 104 ++---
1 file changed, 71 insertions(+), 33 deletions(-)



pgsql: Move libpq encryption negotiation tests

2024-04-12 Thread Heikki Linnakangas
Move libpq encryption negotiation tests

The test targets libpq's options, so 'src/test/interfaces/libpq/t' is
a more natural place for it.

While doing this, I noticed that I had missed adding the
libpq_encryption subdir to the Makefile. That's why this commit only
needs to remove it from the meson.build file.

Per Peter Eisentraut's suggestion.

Discussion: 
https://www.postgresql.org/message-id/09d4bf5d-d0fa-4c66-a1d7-5ec757609...@eisentraut.org

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/65dfe9d167e925cd8892dedb51dde29f69b7388d

Modified Files
--
src/interfaces/libpq/Makefile  |  2 +-
src/interfaces/libpq/meson.build   |  7 -
.../libpq/t/005_negotiate_encryption.pl}   |  2 +-
src/test/libpq_encryption/Makefile | 25 -
src/test/libpq_encryption/README   | 31 --
src/test/libpq_encryption/meson.build  | 18 -
src/test/meson.build   |  1 -
7 files changed, 8 insertions(+), 78 deletions(-)



pgsql: Fix compilation with --with-gssapi --without-openssl

2024-04-12 Thread Heikki Linnakangas
Fix compilation with --with-gssapi --without-openssl

The #define is spelled ENABLE_GSS, not USE_GSS. Introduced in commit
05fd30c0e7, reported by Thomas Munro.

Discussion: 
https://www.postgresql.org/message-id/ca%2bhukg%2bhrttb%2bx%2bkkkj_cfx6snhbeguqmgxjgmwdvpg7ygf...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/084cae55131f71f32e15a6b2b7897aa1c7382c0a

Modified Files
--
src/interfaces/libpq/fe-connect.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



pgsql: Shrink test file for test_json_parser module

2024-04-12 Thread Andrew Dunstan
Shrink test file for test_json_parser module

Also delete live URLs

Jacob Champion

Discussion: 
https://postgr.es/m/CAOYmi+mtH=v1wzkaoaucd5qqqwr61hnxmjbj9h-czxaa1jx...@mail.gmail.com

Branch
--
master

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

Modified Files
--
src/test/modules/test_json_parser/tiny.json |   515 +-
src/test/modules/test_json_parser/tiny.out  | 19346 +-
2 files changed, 192 insertions(+), 19669 deletions(-)



pgsql: Assorted minor cleanups in the test_json_parser module

2024-04-12 Thread Andrew Dunstan
Assorted minor cleanups in the test_json_parser module

Per gripes from Michael Paquier

Discussion: https://postgr.es/m/zhtq6_w1vwohq...@paquier.xyz

Along the way, also clean up a handful of typos in 3311ea86ed and
ea7b4e9a2a, found by Alexander Lakhin, and a couple of stylistic
snafus noted by Daniel Westermann and Daniel Gustafsson.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/42fa4b660143b66bea1fb90793ec90054e170c93

Modified Files
--
src/backend/backup/basebackup_incremental.c|  6 ++---
src/common/jsonapi.c   |  6 ++---
src/common/parse_manifest.c|  2 +-
src/test/modules/test_json_parser/README   | 19 +++
.../test_json_parser_incremental.c | 28 --
.../test_json_parser/test_json_parser_perf.c   | 11 +
6 files changed, 43 insertions(+), 29 deletions(-)



pgsql: Add a TAP test for test_json_parser_perf

2024-04-12 Thread Andrew Dunstan
Add a TAP test for test_json_parser_perf

This just makes sure the test can run with a single iteration. A real
performance test would test with many more.

Branch
--
master

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

Modified Files
--
src/test/modules/test_json_parser/meson.build  |  3 +-
.../test_json_parser/t/004_test_parser_perf.pl | 35 ++
2 files changed, 37 insertions(+), 1 deletion(-)



pgsql: Don't allocate large buffer on the stack in pg_verifybackup

2024-04-12 Thread Andrew Dunstan
Don't allocate large buffer on the stack in pg_verifybackup

Per complaint from Andres Freund. Follow his suggestion to allocate the
buffer once in the calling routine instead.

Also make a tiny indentation improvement.

Discussion: 
https://postgr.es/m/20240411190147.a3yries632olf...@awork3.anarazel.de

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/929c05774b512bdf7ea4a5912fa2f4d63fc90712

Modified Files
--
src/bin/pg_verifybackup/pg_verifybackup.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)



pgsql: Fix some memory leaks associated with parsing json and manifests

2024-04-12 Thread Andrew Dunstan
Fix some memory leaks associated with parsing json and manifests

Coverity complained about not freeing some memory associated with
incrementally parsing backup manifests. To fix that, provide and use a new
shutdown function for the JsonManifestParseIncrementalState object, in
line with a suggestion from Tom Lane.

While analysing the problem, I noticed a buglet in freeing memory for
incremental json lexers. To fix that remove a bogus condition on
freeing the memory allocated for them.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/661ab4e185784db79c194b5758555b1db3f30483

Modified Files
--
src/backend/backup/basebackup_incremental.c |  3 +++
src/bin/pg_combinebackup/load_manifest.c|  3 +++
src/bin/pg_verifybackup/pg_verifybackup.c   |  3 +++
src/common/jsonapi.c| 21 ++---
src/common/parse_manifest.c | 13 -
src/include/common/parse_manifest.h |  1 +
6 files changed, 32 insertions(+), 12 deletions(-)



pgsql: Fix recently introduced typo in code comment

2024-04-12 Thread David Rowley
Fix recently introduced typo in code comment

Reported-by: Richard Guo
Discussion: 
https://postgr.es/m/cambws49kaszusj7-0sblve9+ukz0rcqmemm3nvytc1yvs8s...@mail.gmail.com

Branch
--
master

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

Modified Files
--
src/backend/optimizer/plan/initsplan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



pgsql: Fix the review comments and a bug in the slot sync code.

2024-04-12 Thread Amit Kapila
Fix the review comments and a bug in the slot sync code.

Ensure that when updating the catalog_xmin of the synced slots, it is
first written to disk before changing the in-memory value
(effective_catalog_xmin). This is to prevent a scenario where the
in-memory value change triggers a vacuum to remove catalog tuples before
the catalog_xmin is written to disk. In the event of a crash before the
catalog_xmin is persisted, we would not know that some required catalog
tuples have been removed and the synced slot would be invalidated.

Change the sanity check to ensure that remote_slot's confirmed_flush LSN
can't precede the local/synced slot during slot sync. Note that the
restart_lsn of the synced/local slot can be ahead of remote_slot. This can
happen when slot advancing machinery finds a running xacts record after
reaching the consistent state at a later point than the primary where it
serializes the snapshot and updates the restart_lsn.

Make the check to sync slots robust by allowing to sync only when the
confirmed_lsn, restart_lsn, or catalog_xmin of the remote slot is ahead of
the synced/local slot.

Reported-by: Amit Kapila and Shveta Malik
Author: Hou Zhijie, Shveta Malik
Reviewed-by: Amit Kapila, Bertrand Drouvot
Discussion: 
https://postgr.es/m/os0pr01mb57162b67d3cb01b2756fba6d94...@os0pr01mb5716.jpnprd01.prod.outlook.com
Discussion: 
https://postgr.es/m/cajpy0ucss5zmdyuxhvw41hsdtbrqx1hbyqkofhnj7qq+2zn...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/3741f2a09d5205ec32bd8af5d1f397e08995932b

Modified Files
--
src/backend/replication/logical/slotsync.c | 163 +
1 file changed, 119 insertions(+), 44 deletions(-)



pgsql: Fix IS [NOT] NULL qual optimization for inheritance tables

2024-04-12 Thread David Rowley
Fix IS [NOT] NULL qual optimization for inheritance tables

b262ad440 added code to have the planner remove redundant IS NOT NULL
quals and eliminate needless scans for IS NULL quals on tables where the
qual's column has a NOT NULL constraint.

That commit failed to consider that an inheritance parent table could
have differing NOT NULL constraints between the parent and the child.
This caused issues as if we eliminated a qual on the parent, when
applying the quals to child tables in apply_child_basequals(), the qual
might not have been added to the parent's baserestrictinfo.

Here we fix this by not applying the optimization to remove redundant
quals to RelOptInfos belonging to inheritance parents and applying the
optimization again in apply_child_basequals().  Effectively, this means
that the parent and child are considered independently as the parent has
both an inh=true and inh=false RTE and we still apply the optimization
to the RelOptInfo corresponding to the inh=false RTE.

We're able to still apply the optimization in add_base_clause_to_rel()
for partitioned tables as the NULLability of partitions must match that
of their parent.  And, if we ever expand restriction_is_always_false()
and restriction_is_always_true() to handle partition constraints then we
can apply the same logic as, even in multi-level partitioned tables,
there's no way to route values to a partition when the qual does not
match the partition qual of the partitioned table's parent partition.
The same is true for CHECK constraints as those must also match between
arent partitioned tables and their partitions.

Author: Richard Guo, David Rowley
Discussion: 
https://postgr.es/m/cambws4930gqszmjr7aanzeapdy61gcg6z8dt-kaeyd0sywk...@mail.gmail.com

Branch
--
master

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

Modified Files
--
src/backend/optimizer/plan/initsplan.c  | 60 ++---
src/backend/optimizer/util/inherit.c| 33 --
src/backend/optimizer/util/plancat.c| 37 +---
src/backend/optimizer/util/relnode.c| 25 +-
src/include/nodes/pathnodes.h   |  6 +++-
src/test/regress/expected/predicate.out | 48 ++
src/test/regress/sql/predicate.sql  | 27 +++
7 files changed, 182 insertions(+), 54 deletions(-)