From 9ae38a393febe86eff9471cc6512e46594993e48 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Thu, 18 Jun 2026 14:03:01 +0300 Subject: [PATCH 01/19] Add PostgreSQL 19 (beta1) support This makes pg_lake build and pass tests on PostgreSQL 19beta1, and keeps 16/17/18 working. The majority of the changes are trivial: missing includes, CI matrix bumps, and small API adjustments behind PG_VERSION_NUM guards. A few areas needed real thought, noted below. - COPY (JSON): PG19 added a native COPY ... TO (FORMAT json). We don't want to silently diverge from core, so we give core precedence: for the cases it handles itself (uncompressed COPY TO a local file or STDOUT) we step aside, and pg_lake keeps the rest (COPY FROM json, compression, remote/lake targets). This is the same idea we already use for CSV. The routing sits behind a hidden, test only GUC pg_lake_copy.json_copy_mode (auto/postgres/ pglake); auto is the production default and the upstream regression suite forces postgres so core's expected output reproduces exactly. We also map COPY FORCE_ARRAY to DuckDB's ARRAY writer. On the test side we round-trip every case (write, then read it back through pg_lake), parametrize the local cases over both writers, and pin the real divergences (generated columns, pg_map serialization) so they can't regress unnoticed. - Wire protocol negotiation: PG19 libpq probes whether the server speaks NegotiateProtocolVersion (it requests protocol 3.9999 and sends a fake _pq_.* option). pgduck_server used to drop any connection that was not exactly 3.0, so every PG19 client failed to attach. It now replies with a 'v' message advertising 3.0 and echoes back the unknown _pq_.* options, same as core. Older clients are not affected. - gnu11: PG19 headers use the C11 static_assert keyword (StaticAssertDecl), so -std=gnu99 stopped compiling. We bump the extension Makefiles to -std=gnu11 unconditionally; it is a superset and still builds on 16/17/18. - TupleDescFinalize: PG19 added a cache field to TupleDesc and asserts it is populated before a descriptor is blessed. Every place we build a TupleDesc by hand now calls TupleDescFinalize(), with a no-op shim in pg_compat.h for <=18 so the call can stay unconditional. - Shared memory init: PG19 replaced the shmem_request_hook/shmem_startup_hook pair with RegisterShmemCallbacks plus ShmemRequestStruct/ShmemRequestHash. We use the new path on PG19 and keep the old hooks for <=18, gated by version. - COPY TO partitioned tables: PG19 lets you COPY a partitioned table directly. pg_lake rejected this before. We allow it on PG19+ so a partitioned parent (whose leaves can be pg_lake/iceberg tables) streams out in one statement, and we set the SELECT inh flag from relkind so the parent descends into its partitions. - pgaudit: pgaudit has no PG19 SQL/PGQ (graph_table) support yet, and with it preloaded the backend crashes on the property graph statements in core's own privileges regression test, cascading into ~100 failures. We drop pgaudit from the preload list for installcheck-postgres on PG19 only (it stays on for <=18). It only writes to the server log, so regression output does not change. - pg_cron: upstream pg_cron has no PG19 release, so for CI we build it from CyberDem0n's postgresql-19 branch (citusdata/pg_cron PR #430) until that lands. - pull_var_clause recursion: on PG19 aggregate/placeholder nodes can appear inside baserestrictinfo clauses where earlier majors did not, and pull_var_clause elog()s on them without a recurse flag, so we pass PVC_RECURSE_AGGREGATES at those sites. postgres_fdw never needed this because it does not push down the complex query shapes we do, so it does not reach those clauses the same way. - GROUP BY ALL: PG19's column-inference GROUP BY ALL also exists in DuckDB, but the two infer the grouping columns differently and the deparser keeps the literal "GROUP BY ALL", so pushing it down could let DuckDB pick a different grouping. We mark such queries as not shippable and run them locally with the grouping PostgreSQL already resolved. The older "GROUP BY ALL col" modifier is not affected. - IGNORE NULLS window functions: PG19 added IGNORE NULLS / RESPECT NULLS, and core deparses it as "lag(v) IGNORE NULLS OVER ...", which DuckDB rejects. We mark window funcs with ignore_nulls != NO_NULLTREATMENT as not shippable. RESPECT NULLS is the default and collapses to NO_NULLTREATMENT, so it still pushes down. - Isolation tests: PG19 reports a serialization conflict from a concurrent delete with different wording (and our copy-on-write makes an UPDATE look like a delete to a concurrent reader). Instead of keeping _1.out alternate expected files, we trap the failure by SQLSTATE (40001) and re-raise one canonical message, so a single expected file matches every version. Co-authored-by: Cursor --- .github/actions/run-test/action.yml | 35 +- .github/workflows/pytest_all.yml | 18 +- docker/Dockerfile | 47 +- docker/LOCAL_DEV.md | 6 +- docker/README.md | 2 +- docker/TASKFILE.md | 7 +- docker/Taskfile.yml | 7 +- docs/building-from-source.md | 10 +- install.sh | 10 +- pg_extension_base/Makefile | 2 +- .../include/pg_extension_base/base_workers.h | 3 + .../include/pg_extension_base/pg_compat.h | 68 ++ .../include/pg_extension_base/spi_helpers.h | 2 + pg_extension_base/src/attached_worker.c | 4 + pg_extension_base/src/base_worker_launcher.c | 99 +- pg_extension_base/src/library_preloader.c | 1 + .../pg_extension_base_test_common/Makefile | 2 +- .../pg_extension_base_test_ext1/Makefile | 2 +- .../pg_extension_base_test_ext2/Makefile | 2 +- .../pg_extension_base_test_ext3/Makefile | 2 +- .../pg_extension_base_test_hibernate/Makefile | 2 +- .../pg_extension_base_test_oneshot/Makefile | 2 +- .../pg_extension_base_test_scheduler/Makefile | 2 +- pg_extension_updater/Makefile | 2 +- .../src/pg_extension_updater.c | 1 + pg_lake_benchmark/Makefile | 2 +- pg_lake_benchmark/src/tpcds.c | 1 + pg_lake_benchmark/src/tpch.c | 1 + pg_lake_copy/Makefile | 2 +- pg_lake_copy/include/pg_lake/copy/copy.h | 30 + pg_lake_copy/src/copy/copy.c | 138 ++- pg_lake_copy/src/init.c | 27 + pg_lake_copy/src/test/types.c | 4 + pg_lake_copy/tests/pytests/test_json_copy.py | 325 +++++++ .../tests/pytests/test_json_copy_vs_core.py | 817 ++++++++++++++++ .../tests/pytests/test_parquet_copy.py | 288 +++++- .../tests/pytests/test_pg19_copy_options.py | 122 +++ pg_lake_engine/Makefile | 2 +- .../include/pg_lake/csv/csv_options.h | 9 + .../include/pg_lake/pgduck/serialize.h | 1 + pg_lake_engine/src/cleanup/deletion_queue.c | 2 + .../src/cleanup/in_progress_files.c | 3 + pg_lake_engine/src/csv/csv_writer.c | 22 +- .../src/data_file/data_file_stats.c | 1 + pg_lake_engine/src/json/json_reader.c | 1 + pg_lake_engine/src/parquet/field.c | 1 + pg_lake_engine/src/parsetree/columns.c | 5 + pg_lake_engine/src/parsetree/const.c | 1 + pg_lake_engine/src/parsetree/expression.c | 1 + pg_lake_engine/src/pgduck/client.c | 1 + pg_lake_engine/src/pgduck/map.c | 2 + pg_lake_engine/src/pgduck/parse_struct.c | 1 + pg_lake_engine/src/pgduck/read_data.c | 1 + pg_lake_engine/src/pgduck/rewrite_query.c | 3 + .../src/pgduck/shippable_spatial_functions.c | 1 + pg_lake_engine/src/pgduck/type.c | 1 + pg_lake_engine/src/pgduck/write_data.c | 7 + pg_lake_engine/src/query/execute.c | 6 +- pg_lake_engine/src/utils/compare_utils.c | 1 + pg_lake_engine/src/utils/operator_utils.c | 1 + pg_lake_engine/src/utils/plan_cache.c | 1 + pg_lake_iceberg/Makefile | 2 +- .../src/iceberg/api/table_metadata.c | 1 + pg_lake_iceberg/src/iceberg/catalog.c | 1 + .../src/iceberg/iceberg_type_binary_serde.c | 4 + .../src/iceberg/iceberg_type_json_serde.c | 1 + .../src/iceberg/metadata_operations.c | 1 + .../operations/find_referenced_files.c | 1 + pg_lake_iceberg/src/iceberg/read_manifest.c | 3 + .../object_store_catalog.c | 2 + pg_lake_iceberg/src/test/test_http_client.c | 1 + .../src/test/test_iceberg_binary_serde.c | 2 + .../src/test/test_iceberg_field_ids.c | 1 + .../src/test/test_iceberg_manifest.c | 2 + pg_lake_iceberg/src/utils/truncate_utils.c | 1 + .../include/pg_lake/fdw/data_files_catalog.h | 2 + pg_lake_table/include/pg_lake/fdw/shippable.h | 1 + pg_lake_table/src/ddl/alter_table.c | 1 + pg_lake_table/src/ddl/create_table.c | 1 + pg_lake_table/src/describe/describe.c | 1 + .../src/duckdb/transform_query_to_duckdb.c | 1 + pg_lake_table/src/fdw/data_file_pruning.c | 13 +- pg_lake_table/src/fdw/data_files_catalog.c | 2 + .../src/fdw/data_files_catalog_batch.c | 4 + pg_lake_table/src/fdw/deparse.c | 10 +- pg_lake_table/src/fdw/deparse_ruleutils.c | 1 + pg_lake_table/src/fdw/partition_pushdown.c | 1 + pg_lake_table/src/fdw/partition_transform.c | 2 + .../fdw/partitioning/partition_spec_catalog.c | 2 + .../partitioning/partitioned_dest_receiver.c | 1 + pg_lake_table/src/fdw/pg_lake_table.c | 38 +- pg_lake_table/src/fdw/position_delete_dest.c | 2 + .../field_id_mapping_catalog.c | 1 + .../schema_operations/register_field_ids.c | 21 +- pg_lake_table/src/fdw/shippable.c | 2 + pg_lake_table/src/fdw/update_tracking.c | 6 +- pg_lake_table/src/fdw/writable_table.c | 6 + pg_lake_table/src/planner/explain.c | 1 + pg_lake_table/src/planner/insert_select.c | 1 + pg_lake_table/src/planner/query_pushdown.c | 57 +- .../src/planner/restriction_collector.c | 12 +- pg_lake_table/src/recovery/recover.c | 1 + pg_lake_table/src/test/hide_lake_objects.c | 2 + pg_lake_table/src/test/test_partition_tuple.c | 3 + .../track_iceberg_metadata_changes.c | 1 + .../isolation_iceberg_repeatable_read.out | 48 +- .../isolation_iceberg_serializable.out | 24 +- .../isolation_iceberg_repeatable_read.spec | 30 +- .../specs/isolation_iceberg_serializable.spec | 24 +- .../pytests/test_grouping_set_pushdown.py | 54 ++ .../tests/pytests/test_pg19_new_features.py | 911 ++++++++++++++++++ .../tests/pytests/test_pgduck_server_crash.py | 26 +- .../Makefile | 2 +- .../pg_lake_table_test_hideable/Makefile | 2 +- pg_map/src/map.c | 9 + pgduck_server/Makefile | 6 +- pgduck_server/include/utils/numutils.h | 2 - pgduck_server/src/duckdb/type_conversion.c | 1 + pgduck_server/src/pgsession/pgsession_io.c | 67 +- pgduck_server/tests/ctests/Makefile | 6 +- .../pytests/test_protocol_negotiation.py | 159 +++ pgduck_server/tests/pytests/utils_protocol.py | 88 +- 122 files changed, 3717 insertions(+), 132 deletions(-) create mode 100644 pg_lake_copy/tests/pytests/test_json_copy_vs_core.py create mode 100644 pg_lake_copy/tests/pytests/test_pg19_copy_options.py create mode 100644 pg_lake_table/tests/pytests/test_pg19_new_features.py create mode 100644 pgduck_server/tests/pytests/test_protocol_negotiation.py diff --git a/.github/actions/run-test/action.yml b/.github/actions/run-test/action.yml index a148b4325..867f88911 100644 --- a/.github/actions/run-test/action.yml +++ b/.github/actions/run-test/action.yml @@ -102,7 +102,21 @@ runs: if [ ${{ inputs.installcheck }} == 'true' ]; then pgduck_server --init_file_path pgduck_server/tests/test_secrets.sql --extensions_dir /tmp/extensions --cache_dir /tmp/cache > /tmp/pgduck_installcheck_logs 2>&1 & - initdb -D /tmp/pg_installcheck_tests -U postgres --set "shared_preload_libraries=pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" --locale=C.UTF-8 --data-checksums + # pgaudit has not yet added support for PostgreSQL 19's SQL/PGQ + # (graph_table); with pgaudit preloaded the backend crashes on the + # property-graph statements in PostgreSQL's own privileges regression + # test, cascading into the rest of the parallel group. Drop pgaudit + # from the preload list on PG19+ only (it stays on <=18). pgaudit only + # emits server-log entries, so this never changes regression output. + PG_MAJOR="${{ inputs.pg_version }}" + PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" + PGAUDIT_ENABLED=true + if [[ "$PG_MAJOR" =~ ^[0-9]+$ ]] && [ "$PG_MAJOR" -ge 19 ]; then + PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pg_stat_statements" + PGAUDIT_ENABLED=false + fi + + initdb -D /tmp/pg_installcheck_tests -U postgres --set "shared_preload_libraries=$PRELOAD_LIBS" --locale=C.UTF-8 --data-checksums echo "listen_addresses = 'localhost'" >> /tmp/pg_installcheck_tests/postgresql.conf echo "max_worker_processes = 100" >> /tmp/pg_installcheck_tests/postgresql.conf @@ -111,11 +125,28 @@ runs: echo "pg_lake_iceberg.autovacuum_naptime=5" >> /tmp/pg_installcheck_tests/postgresql.conf echo "unix_socket_directories = '/tmp/pg_installcheck_tests'" >> /tmp/pg_installcheck_tests/postgresql.conf echo "max_prepared_transactions = 100" >> /tmp/pg_installcheck_tests/postgresql.conf - echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf + if [ "$PGAUDIT_ENABLED" == "true" ]; then + echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf + fi if [ ${{ inputs.installcheck_postgres }} == 'true' ]; then echo "compute_query_id='regress'" >> /tmp/pg_installcheck_tests/postgresql.conf echo "pg_lake_table.hide_objects_created_by_lake=true" >> /tmp/pg_installcheck_tests/postgresql.conf + # Fully defer JSON COPY to Postgres for the upstream regression + # suite. The surgical "auto" default already routes plain local + # COPY TO json to Postgres, but pg_lake still keeps COPY FROM json + # and compression (things Postgres cannot do) -- and those diverge + # from the upstream expected output, which is real (not whitespace) + # and is covered by pg_lake_copy's own tests rather than + # PostgreSQL's: + # - pg_lake accepts COPY FROM ... (format json), which Postgres + # rejects, so the inline-data copy test desyncs; + # - it rejects the ENCODING option and mishandles a non-UTF8 + # client_encoding (copyencoding); + # - it frames FORCE_ARRAY output differently (tabs/trailing comma). + # Forcing json_copy_mode=postgres makes pg_lake step fully aside so + # those upstream tests stay byte-identical to Postgres. + echo "pg_lake_copy.json_copy_mode=postgres" >> /tmp/pg_installcheck_tests/postgresql.conf fi echo "host postgres postgres 127.0.0.1/32 md5" >> /tmp/pg_installcheck_tests/pg_hba.conf diff --git a/.github/workflows/pytest_all.yml b/.github/workflows/pytest_all.yml index 578ce8250..1bf488a79 100644 --- a/.github/workflows/pytest_all.yml +++ b/.github/workflows/pytest_all.yml @@ -83,7 +83,6 @@ jobs: build-and-install-almalinux: needs: build-image-almalinux runs-on: ubuntu-latest - container: image: ghcr.io/snowflake-labs/pg-lake-ci-pg-almalinux:${{ needs.build-image-almalinux.outputs.tag }} options: --user 1001 --shm-size=128MB @@ -92,8 +91,9 @@ jobs: password: ${{ secrets.github_token }} strategy: + fail-fast: false matrix: - pg_version: [16, 17, 18] + pg_version: [16, 17, 18, 19] steps: - uses: actions/checkout@v4 @@ -108,7 +108,6 @@ jobs: build-and-install-debian: needs: build-image-debian runs-on: ubuntu-latest - container: image: ghcr.io/snowflake-labs/pg-lake-ci-pg-debian:${{ needs.build-image-debian.outputs.tag }} options: --user 1001 --shm-size=128MB @@ -117,8 +116,9 @@ jobs: password: ${{ secrets.github_token }} strategy: + fail-fast: false matrix: - pg_version: [16, 17, 18] + pg_version: [16, 17, 18, 19] steps: - uses: actions/checkout@v4 @@ -151,7 +151,7 @@ jobs: strategy: fail-fast: false matrix: - pg_version: [17, 18] + pg_version: [17, 18, 19] test_make_target: - 'check-duckdb_pglake' - 'check-avro' @@ -189,7 +189,7 @@ jobs: strategy: fail-fast: false matrix: - pg_version: [17, 18] + pg_version: [17, 18, 19] test_make_target: - 'check-pg_lake_table' # helps splitting tests into 5 parallel jobs @@ -257,7 +257,7 @@ jobs: strategy: fail-fast: false matrix: - pg_version: [16, 17, 18] + pg_version: [16, 17, 18, 19] test_make_target: - 'installcheck-postgres' - 'installcheck-postgres-with_extensions_created' @@ -329,7 +329,7 @@ jobs: strategy: fail-fast: false matrix: - pg_version: [17, 18] + pg_version: [17, 18, 19] test_make_target: - 'check-isolation_pg_lake_table' @@ -358,7 +358,7 @@ jobs: strategy: fail-fast: false matrix: - pg_version_pairs: ["16-17", "17-18", "16-18"] + pg_version_pairs: ["16-17", "17-18", "16-18", "17-19", "18-19", "16-19"] test_make_target: - 'check-pg_lake_table-upgrade' diff --git a/docker/Dockerfile b/docker/Dockerfile index 94214a8e1..6912cb26f 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -9,13 +9,15 @@ ARG BASE_IMAGE_TAG # Set environment variables for non-interactive installations and PostgreSQL/PostGIS versions ENV PG_LAKE_REF=main -ARG PG16_VERSION=16.11 -ARG PG17_VERSION=17.7 -ARG PG18_VERSION=18.1 -ARG POSTGIS_VERSION=3.6.0 +ARG PG16_VERSION=16.14 +ARG PG17_VERSION=17.10 +ARG PG18_VERSION=18.4 +ARG PG19_VERSION=19beta1 +ARG POSTGIS_VERSION=3.6.3 ARG PGAUDIT16_VERSION=REL_16_STABLE ARG PGAUDIT17_VERSION=REL_17_STABLE ARG PGAUDIT18_VERSION=REL_18_STABLE +ARG PGAUDIT19_VERSION=main # Install build dependencies RUN if [ "$BASE_IMAGE_OS" = "almalinux" ]; then \ @@ -28,6 +30,7 @@ RUN if [ "$BASE_IMAGE_OS" = "almalinux" ]; then \ ninja-build \ wget \ git \ + bzip2 \ readline-devel \ zlib-devel \ flex \ @@ -73,6 +76,7 @@ RUN if [ "$BASE_IMAGE_OS" = "debian" ]; then \ apt-get update \ && apt-get install -y \ build-essential \ + bzip2 \ cmake \ ninja-build \ libreadline-dev \ @@ -128,7 +132,8 @@ RUN if [ "$BASE_IMAGE_OS" = "debian" ]; then \ wget https://download.oracle.com/java/21/latest/jdk-21_linux-${ARCH}_bin.tar.gz && \ mkdir -p /usr/lib/jvm && \ tar -xzf jdk-21_linux-${ARCH}_bin.tar.gz -C /usr/lib/jvm/ && \ - mv /usr/lib/jvm/jdk-21.0.10 /usr/lib/jvm/java-21-openjdk && \ + JDK_DIR=$(tar -tzf jdk-21_linux-${ARCH}_bin.tar.gz | head -1 | cut -d/ -f1) && \ + mv "/usr/lib/jvm/${JDK_DIR}" /usr/lib/jvm/java-21-openjdk && \ rm jdk-21_linux-${ARCH}_bin.tar.gz; \ fi @@ -157,26 +162,31 @@ RUN echo "postgres ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers.d/postgres USER 1001:1001 WORKDIR /home/postgres -# Download PostgreSQL 16, 17, 18, PostGIS +# Download PostgreSQL 16, 17, 18, 19, PostGIS RUN wget https://ftp.postgresql.org/pub/source/v$PG16_VERSION/postgresql-$PG16_VERSION.tar.gz && \ wget https://ftp.postgresql.org/pub/source/v$PG17_VERSION/postgresql-$PG17_VERSION.tar.gz && \ wget https://ftp.postgresql.org/pub/source/v$PG18_VERSION/postgresql-$PG18_VERSION.tar.gz && \ + wget https://ftp.postgresql.org/pub/source/v$PG19_VERSION/postgresql-$PG19_VERSION.tar.bz2 && \ wget https://download.osgeo.org/postgis/source/postgis-$POSTGIS_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT16_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT17_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT18_VERSION.tar.gz && \ + wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ tar -xzf postgresql-$PG16_VERSION.tar.gz && rm postgresql-$PG16_VERSION.tar.gz && \ tar -xzf postgresql-$PG17_VERSION.tar.gz && rm postgresql-$PG17_VERSION.tar.gz && \ tar -xzf postgresql-$PG18_VERSION.tar.gz && rm postgresql-$PG18_VERSION.tar.gz && \ + tar -xjf postgresql-$PG19_VERSION.tar.bz2 && rm postgresql-$PG19_VERSION.tar.bz2 && \ tar -xzf $PGAUDIT16_VERSION.tar.gz && rm $PGAUDIT16_VERSION.tar.gz && \ tar -xzf $PGAUDIT17_VERSION.tar.gz && rm $PGAUDIT17_VERSION.tar.gz && \ tar -xzf $PGAUDIT18_VERSION.tar.gz && rm $PGAUDIT18_VERSION.tar.gz && \ + tar -xzf $PGAUDIT19_VERSION.tar.gz && rm $PGAUDIT19_VERSION.tar.gz && \ tar -xzf postgis-$POSTGIS_VERSION.tar.gz && rm postgis-$POSTGIS_VERSION.tar.gz ARG PG_COMPILE_FLAGS_16="--enable-debug --enable-cassert --enable-depend --with-openssl --with-libxml --with-libxslt --with-icu --with-uuid=ossp --with-lz4 --with-pam" # PG17 has asserts disabled ARG PG_COMPILE_FLAGS_17="--enable-debug --enable-depend --with-openssl --with-libxml --with-libxslt --with-icu --with-uuid=ossp --with-lz4 --enable-injection-points --with-pam" ARG PG_COMPILE_FLAGS_18="--enable-debug --enable-cassert --enable-depend --with-openssl --with-libxml --with-libxslt --with-icu --with-uuid=ossp --with-lz4 --enable-injection-points --with-pam" +ARG PG_COMPILE_FLAGS_19="--enable-debug --enable-cassert --enable-depend --with-openssl --with-libxml --with-libxslt --with-icu --with-uuid=ossp --with-lz4 --enable-injection-points --with-pam" ENV PGBASEDIR=/home/postgres # Compile and install PostgreSQL 16 with pgindent @@ -217,6 +227,19 @@ RUN cd postgresql-$PG18_VERSION && \ (cd contrib && make -j `nproc` install) && \ cd .. && rm -rf postgresql-$PG18_VERSION* +# Compile and install PostgreSQL 19 with pgindent +RUN cd postgresql-$PG19_VERSION && \ + ./configure --prefix=$PGBASEDIR/pgsql-19 $PG_COMPILE_FLAGS_19 && \ + make -j `nproc` && make install && \ + make -C src/test/isolation install && \ + make -C src/test/modules/injection_points install && \ + make -j `nproc` -C src/tools/pg_bsd_indent install && \ + cp src/tools/pgindent/pgindent $PGBASEDIR/pgsql-19/bin/ && \ + cp src/tools/pgindent/typedefs.list $PGBASEDIR/pgsql-19/share/ && \ + mkdir -pv $PGBASEDIR/pgsql-19/regress && cp -r src/test/regress/* $PGBASEDIR/pgsql-19/regress && \ + (cd contrib && make -j `nproc` install) && \ + cd .. && rm -rf postgresql-$PG19_VERSION* + # Compile and install PG AUDIT 16 RUN cd pgaudit-$PGAUDIT16_VERSION && \ make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config @@ -229,19 +252,28 @@ RUN cd pgaudit-$PGAUDIT17_VERSION && \ RUN cd pgaudit-$PGAUDIT18_VERSION && \ make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config +# Compile and install PG AUDIT 19 +RUN cd pgaudit-$PGAUDIT19_VERSION && \ + make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config + # Compile and install PostGIS RUN cd postgis-$POSTGIS_VERSION && \ ./configure --prefix=$PGBASEDIR/pgsql-16/ --with-pgconfig=$PGBASEDIR/pgsql-16/bin/pg_config && make -j `nproc` && make install && make clean && \ ./configure --prefix=$PGBASEDIR/pgsql-17/ --with-pgconfig=$PGBASEDIR/pgsql-17/bin/pg_config && make -j `nproc` && make install && make clean && \ ./configure --prefix=$PGBASEDIR/pgsql-18/ --with-pgconfig=$PGBASEDIR/pgsql-18/bin/pg_config && make -j `nproc` && make install && make clean && \ + ./configure --prefix=$PGBASEDIR/pgsql-19/ --with-pgconfig=$PGBASEDIR/pgsql-19/bin/pg_config && make -j `nproc` && make install && make clean && \ cd .. && rm -rf postgis-$POSTGIS_VERSION* # Compile and install pg_cron -RUN git clone https://github.com/citusdata/pg_cron.git && \ +# Use CyberDem0n's postgresql-19 branch (PR citusdata/pg_cron#430) until the +# PG19 support patch is merged upstream. The patch is backward compatible +# with PG 16/17/18, so we can build all four from the same source. +RUN git clone -b postgresql-19 https://github.com/CyberDem0n/pg_cron.git && \ cd pg_cron && \ make install PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config && \ make install PG_CONFIG=$PGBASEDIR/pgsql-17/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-17/bin/pg_config && \ make install PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config && \ + make install PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config && \ cd .. && rm -rf pg_cron # Compile and install pg_incremental @@ -250,6 +282,7 @@ RUN git clone https://github.com/CrunchyData/pg_incremental.git && \ make install PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config && \ make install PG_CONFIG=$PGBASEDIR/pgsql-17/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-17/bin/pg_config && \ make install PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config && \ + make install PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config && make clean PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config && \ cd .. && rm -rf pg_incremental # Install vcpkg as postgres user diff --git a/docker/LOCAL_DEV.md b/docker/LOCAL_DEV.md index 69c8815c5..5963a3e14 100644 --- a/docker/LOCAL_DEV.md +++ b/docker/LOCAL_DEV.md @@ -80,7 +80,7 @@ You can customize builds using these variables (can be set as environment variab | Variable | Default | Description | Example | |----------|---------|-------------|---------| -| `PG_MAJOR` | `18` | PostgreSQL major version (16, 17, or 18) | `PG_MAJOR=17` | +| `PG_MAJOR` | `18` | PostgreSQL major version (16, 17, 18, or 19) | `PG_MAJOR=17` | | `BASE_IMAGE_OS` | `almalinux` | Base OS (almalinux or debian) | `BASE_IMAGE_OS=debian` | | `BASE_IMAGE_TAG` | `9` | Base OS version tag | `BASE_IMAGE_TAG=12` | | `VERSION` | `latest` | Image version tag (for registry) | `VERSION=v1.0.0` | @@ -311,7 +311,7 @@ task compose:up PG_MAJOR=16 Create a `.env` file in the `docker` directory: ```env -# PostgreSQL Version (16, 17, or 18) +# PostgreSQL Version (16, 17, 18, or 19) PG_MAJOR=18 # AWS Profile for LocalStack (optional) @@ -559,6 +559,7 @@ Each PostgreSQL version uses its own buildx builder: - PG 16: `pg_lake_builder_pg16` - PG 17: `pg_lake_builder_pg17` - PG 18: `pg_lake_builder_pg18` +- PG 19: `pg_lake_builder_pg19` This prevents cache conflicts when switching between versions. @@ -590,6 +591,7 @@ This prevents cache conflicts when switching between versions. - PG 16: `pg_lake_builder_pg16` - PG 17: `pg_lake_builder_pg17` - PG 18: `pg_lake_builder_pg18` + - PG 19: `pg_lake_builder_pg19` --- diff --git a/docker/README.md b/docker/README.md index 6ed386e0a..6ac09aa1a 100644 --- a/docker/README.md +++ b/docker/README.md @@ -103,7 +103,7 @@ docker-compose exec pgduck-server psql -p 5332 -h /home/postgres/pgduck_socket_d ### Key Optimizations -✅ **Single Runner PostgreSQL Version**: Builds only PG 16, 17, or 18 (not all 3) +✅ **Single Runner PostgreSQL Version**: Builds only PG 16, 17, 18, or 19 (not all 4) ✅ **Separate Build Stages**: Compilation happens in builder stages ✅ **Minimal Runtime**: Final images contain only binaries and libraries ✅ **Network Retry Logic**: Handles vcpkg download failures diff --git a/docker/TASKFILE.md b/docker/TASKFILE.md index c862efafb..070a0ebed 100644 --- a/docker/TASKFILE.md +++ b/docker/TASKFILE.md @@ -29,6 +29,7 @@ This Taskfile uses **per-version buildx builders** to prevent cache conflicts: - **PG 16**: Uses `pg_lake_builder_pg16` - **PG 17**: Uses `pg_lake_builder_pg17` - **PG 18**: Uses `pg_lake_builder_pg18` +- **PG 19**: Uses `pg_lake_builder_pg19` Each builder maintains its own isolated cache, allowing you to build and switch between PostgreSQL versions without cache conflicts. Builders are created automatically on first use. @@ -186,7 +187,7 @@ task compose:restart ### Cache Management -Each PostgreSQL version (16, 17, 18) uses its own isolated buildx builder to prevent cache conflicts: +Each PostgreSQL version (16, 17, 18, 19) uses its own isolated buildx builder to prevent cache conflicts: ```bash # Clear cache for specific PostgreSQL version @@ -239,7 +240,7 @@ You can customize builds using these variables: | `REGISTRY` | `docker.io` | Container registry (docker.io, ghcr.io, etc.) | | `IMAGE_OWNER` | `${DOCKER_HUB_USERNAME}` | Registry username/org | | `VERSION` | `latest` | Image version tag | -| `PG_MAJOR` | `18` | PostgreSQL major version (16, 17, or 18) | +| `PG_MAJOR` | `18` | PostgreSQL major version (16, 17, 18, or 19) | | `PLATFORMS` | `linux/amd64,linux/arm64` | Target platforms for registry builds | | `BASE_IMAGE_OS` | `almalinux` | Base OS (almalinux or debian) | | `BASE_IMAGE_TAG` | `9` | Base OS version | @@ -510,7 +511,7 @@ task push:all-pg-versions VERSION=v3.1.0 - `build:pg-lake-postgres` - Build pg_lake_postgres for registry (multi-platform, cache only unless PUSH=true) - `build:pgduck-server` - Build pgduck_server for registry (multi-platform, cache only unless PUSH=true) - `build:all` - Build all images (multi-platform, cache only unless PUSH=true) -- `build:all-pg-versions` - Build for PostgreSQL 16, 17, and 18 +- `build:all-pg-versions` - Build for PostgreSQL 16, 17, 18, and 19 ### Push Tasks diff --git a/docker/Taskfile.yml b/docker/Taskfile.yml index 2d1d3c3d6..645389f6f 100644 --- a/docker/Taskfile.yml +++ b/docker/Taskfile.yml @@ -139,7 +139,7 @@ tasks: } build:all-pg-versions: - desc: Build all images for PostgreSQL 16, 17, and 18 + desc: Build all images for PostgreSQL 16, 17, 18, and 19 silent: true cmds: - task: build:all @@ -148,9 +148,11 @@ tasks: vars: { PG_MAJOR: "17", VERSION: "{{.VERSION}}", PUSH: "{{.PUSH}}" } - task: build:all vars: { PG_MAJOR: "18", VERSION: "{{.VERSION}}", PUSH: "{{.PUSH}}" } + - task: build:all + vars: { PG_MAJOR: "19", VERSION: "{{.VERSION}}", PUSH: "{{.PUSH}}" } push:all-pg-versions: - desc: Build and push all images for PostgreSQL 16, 17, and 18 + desc: Build and push all images for PostgreSQL 16, 17, 18, and 19 silent: true cmds: - task: build:all-pg-versions @@ -170,6 +172,7 @@ tasks: - docker buildx rm pg_lake_builder_pg16 || true - docker buildx rm pg_lake_builder_pg17 || true - docker buildx rm pg_lake_builder_pg18 || true + - docker buildx rm pg_lake_builder_pg19 || true - docker buildx rm pg_lake_builder || true - echo "✅ All buildx builders removed" ignore_error: true diff --git a/docs/building-from-source.md b/docs/building-from-source.md index 654484ab6..597b6bd5c 100644 --- a/docs/building-from-source.md +++ b/docs/building-from-source.md @@ -4,7 +4,7 @@ This guide covers installing pg_lake from source. For Docker-based setup, see [d ## Add pg_lake to an Existing PostgreSQL Installation -If you already have PostgreSQL 16, 17, or 18 installed, you can add pg_lake extensions using the automated installation script: +If you already have PostgreSQL 16, 17, 18, or 19 installed, you can add pg_lake extensions using the automated installation script: ```bash # Clone the repository @@ -31,7 +31,7 @@ psql -c "CREATE EXTENSION pg_lake CASCADE;" **Note:** System build dependencies are skipped by default (assuming they're installed if PostgreSQL exists). Use `--with-system-deps` if needed. **Prerequisites:** -- PostgreSQL 16, 17, or 18 installed with `pg_config` in your PATH +- PostgreSQL 16, 17, 18, or 19 installed with `pg_config` in your PATH - `shared_preload_libraries = 'pg_extension_base'` in postgresql.conf ([see setup](#configure-postgresql)) For manual installation steps, see [Manual Installation to Existing PostgreSQL](#manual-installation-to-existing-postgresql). @@ -66,7 +66,7 @@ For more options and control, see [Development Environment Options](#development ## Prerequisites **For adding to existing PostgreSQL:** -- PostgreSQL 16, 17, or 18 with `pg_config` in PATH +- PostgreSQL 16, 17, 18, or 19 with `pg_config` in PATH - Git, C/C++ compiler, CMake, Ninja - Internet connection @@ -219,7 +219,7 @@ Run `./install.sh --help` to see all available options: ``` --build-postgres Build PostgreSQL from source and initialize database ---pg-version VERSION PostgreSQL version to build (16, 17, or 18) [default: 18] +--pg-version VERSION PostgreSQL version to build (16, 17, 18, or 19) [default: 18] --prefix DIR PostgreSQL installation prefix [default: auto-detect or $HOME/pgsql] --deps-dir DIR Directory for dependencies [default: $HOME/pg_lake-deps] --jobs N Number of parallel build jobs [default: nproc] @@ -376,7 +376,7 @@ export PKG_CONFIG_PATH="/opt/homebrew/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/ ### Build PostgreSQL from Source -pg_lake is supported with PostgreSQL 16, 17, and 18. For development, we recommend building PostgreSQL from source to get debug symbols and assertions. +pg_lake is supported with PostgreSQL 16, 17, 18, and 19. For development, we recommend building PostgreSQL from source to get debug symbols and assertions. ```bash # Clone PostgreSQL (version 18 example) diff --git a/install.sh b/install.sh index b8a887169..0ad155679 100755 --- a/install.sh +++ b/install.sh @@ -63,7 +63,7 @@ PostgreSQL installation. Use --build-postgres for full development setup. OPTIONS: --build-postgres Build PostgreSQL from source and initialize database - --pg-version VERSION PostgreSQL version to build (16, 17, or 18) [default: 18] + --pg-version VERSION PostgreSQL version to build (16, 17, 18, or 19) [default: 18] --prefix DIR PostgreSQL installation prefix [default: auto-detect or \$HOME/pgsql] --deps-dir DIR Directory for dependencies [default: \$HOME/pg_lake-deps] --jobs N Number of parallel build jobs [default: nproc] @@ -148,8 +148,8 @@ while [[ $# -gt 0 ]]; do done # Validate PostgreSQL version -if [[ ! "$PG_VERSION" =~ ^(16|17|18)$ ]]; then - print_error "Invalid PostgreSQL version: $PG_VERSION. Must be 16, 17, or 18." +if [[ ! "$PG_VERSION" =~ ^(16|17|18|19)$ ]]; then + print_error "Invalid PostgreSQL version: $PG_VERSION. Must be 16, 17, 18, or 19." exit 1 fi @@ -158,6 +158,10 @@ case $PG_VERSION in 16) PG_BRANCH="REL_16_STABLE" ;; 17) PG_BRANCH="REL_17_STABLE" ;; 18) PG_BRANCH="REL_18_STABLE" ;; + # PG19 has no stable branch yet; pin the beta tag so local builds match + # CI (which builds from the 19beta1 release tarball) instead of tracking + # the moving master branch. + 19) PG_BRANCH="REL_19_BETA1" ;; esac # Determine PostgreSQL installation paths diff --git a/pg_extension_base/Makefile b/pg_extension_base/Makefile index 975ffb8ed..8cf36ddbd 100644 --- a/pg_extension_base/Makefile +++ b/pg_extension_base/Makefile @@ -8,7 +8,7 @@ OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) HEADERS = $(wildcard include/pg_extension_base/*.h) -PG_CPPFLAGS = -Iinclude -std=gnu99 -Wall -Wextra -Wno-unused-parameter +PG_CPPFLAGS = -Iinclude -std=gnu11 -Wall -Wextra -Wno-unused-parameter override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) PG_CONFIG ?= pg_config diff --git a/pg_extension_base/include/pg_extension_base/base_workers.h b/pg_extension_base/include/pg_extension_base/base_workers.h index 705a470dc..b831c7505 100644 --- a/pg_extension_base/include/pg_extension_base/base_workers.h +++ b/pg_extension_base/include/pg_extension_base/base_workers.h @@ -104,8 +104,11 @@ extern bool EnableBaseWorkerLauncher; #define DEFAULT_WORKER_STARTER_SLEEP_TIME (10) extern int WorkerStarterSleepTimeSec; +#if PG_VERSION_NUM < 190000 +/* legacy shmem hook path; PG19+ uses RegisterShmemCallbacks instead */ void BaseWorkerSharedMemoryInit(void); size_t BaseWorkerSharedMemorySize(void); +#endif extern PGDLLEXPORT int32 RegisterBaseWorker(char *workerName, Oid entryPointFunctionId, Oid extensionId); diff --git a/pg_extension_base/include/pg_extension_base/pg_compat.h b/pg_extension_base/include/pg_extension_base/pg_compat.h index 75791c554..562c3a107 100644 --- a/pg_extension_base/include/pg_extension_base/pg_compat.h +++ b/pg_extension_base/include/pg_extension_base/pg_compat.h @@ -18,6 +18,74 @@ #pragma once #include "postgres.h" +#include "access/tupdesc.h" +#include "catalog/pg_type_d.h" +#include "commands/copy.h" +#include "nodes/miscnodes.h" +#include "utils/numeric.h" + +/* + * PG19 added a firstNonCachedOffsetAttr cache to TupleDesc that must be + * populated via TupleDescFinalize() before BlessTupleDesc() or + * heap_form_tuple() are called on a manually-constructed descriptor; PG19's + * BlessTupleDesc Asserts on a missing finalize. Older releases don't expose + * the function, so define a no-op shim there and let callers always finalize. + */ +#if PG_VERSION_NUM < 190000 +static inline void +TupleDescFinalize(TupleDesc tupdesc) +{ +} +#endif + +#if PG_VERSION_NUM >= 190000 + +/* + * PG19 reshaped CopyFormatOptions: the bool fields ".binary" and ".csv_mode" + * were replaced by a single CopyFormat enum field ".format". Provide + * accessor macros so callers keep the older names. + */ +#define CopyOptsIsBinary(opts) ((opts).format == COPY_FORMAT_BINARY) +#define CopyOptsIsCsvMode(opts) ((opts).format == COPY_FORMAT_CSV) + +/* + * PG19 renamed numeric_int4_opt_error -> numeric_int4_safe and switched the + * out-of-band error signal from a bool* to an ErrorSaveContext* (the standard + * "soft error" API). Wrap with the older name + bool* signature. + */ +static inline int32 +numeric_int4_opt_error(Numeric num, bool *have_error) +{ + ErrorSaveContext escontext; + int32 result; + + memset(&escontext, 0, sizeof(escontext)); + escontext.type = T_ErrorSaveContext; + result = numeric_int4_safe(num, (Node *) &escontext); + + *have_error = escontext.error_occurred; + return result; +} + +/* + * Likewise for the binary numeric arithmetic helpers, which were also + * renamed from foo_opt_error(Numeric, Numeric, bool *) to + * foo_safe(Numeric, Numeric, Node *escontext) in PG19. Our callers always + * pass NULL for the error argument, so just forward to the new name. + */ +#define numeric_add_opt_error(num1, num2, _ignored) numeric_add_safe(num1, num2, NULL) +#define numeric_sub_opt_error(num1, num2, _ignored) numeric_sub_safe(num1, num2, NULL) +#define numeric_mul_opt_error(num1, num2, _ignored) numeric_mul_safe(num1, num2, NULL) +#define numeric_div_opt_error(num1, num2, _ignored) numeric_div_safe(num1, num2, NULL) +#define numeric_mod_opt_error(num1, num2, _ignored) numeric_mod_safe(num1, num2, NULL) + +#else + +#define CopyOptsIsBinary(opts) ((opts).binary) +#define CopyOptsIsCsvMode(opts) ((opts).csv_mode) + +#endif + #if PG_VERSION_NUM < 170000 #define foreach_ptr(type, var, lst) foreach_internal(type, *, var, lst, lfirst) diff --git a/pg_extension_base/include/pg_extension_base/spi_helpers.h b/pg_extension_base/include/pg_extension_base/spi_helpers.h index d8a15b875..7359b9b08 100644 --- a/pg_extension_base/include/pg_extension_base/spi_helpers.h +++ b/pg_extension_base/include/pg_extension_base/spi_helpers.h @@ -18,6 +18,7 @@ #ifndef SPI_UTILITIES_H #define SPI_UTILITIES_H +#include "catalog/pg_type_d.h" #include "miscadmin.h" #include "executor/spi.h" @@ -25,6 +26,7 @@ #include "utils/guc.h" #include "utils/jsonb.h" #include "utils/pg_lsn.h" +#include "utils/timestamp.h" /* SPI macros for setting parameters */ #define DATUMIZE_TEXTOID(Value) CStringGetTextDatum(Value) diff --git a/pg_extension_base/src/attached_worker.c b/pg_extension_base/src/attached_worker.c index f580a8e32..097e2f785 100644 --- a/pg_extension_base/src/attached_worker.c +++ b/pg_extension_base/src/attached_worker.c @@ -29,6 +29,7 @@ #include "miscadmin.h" #include "pgstat.h" +#include "access/htup_details.h" #include "access/xact.h" #include "catalog/pg_authid.h" #include "commands/dbcommands.h" @@ -61,6 +62,7 @@ #include "utils/syscache.h" #include "utils/timeout.h" #include "utils/typcache.h" +#include "utils/tuplestore.h" #define QUEUE_SIZE ((Size) 65536) #define ATTACHED_WORKER_MAGIC 0x52004040 @@ -515,6 +517,8 @@ ProcessProtocolMessages(shm_mq_handle *queue, bool nowait, typeMod, 0); } + TupleDescFinalize(tupleDesc); + *resultDesc = tupleDesc; } break; diff --git a/pg_extension_base/src/base_worker_launcher.c b/pg_extension_base/src/base_worker_launcher.c index 393cac5c6..9e064a1d5 100644 --- a/pg_extension_base/src/base_worker_launcher.c +++ b/pg_extension_base/src/base_worker_launcher.c @@ -58,6 +58,9 @@ * should still exist, or whether to clean up their shared memory records. */ #include "postgres.h" +#include "utils/hsearch.h" +#include "access/htup_details.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "miscadmin.h" @@ -94,6 +97,10 @@ #include "utils/memutils.h" #include "utils/snapmgr.h" #include "utils/syscache.h" +#include "utils/tuplestore.h" +#if PG_VERSION_NUM >= 190000 +#include "utils/wait_event.h" +#endif #include "tcop/utility.h" #include "pg_extension_base/base_workers.h" @@ -292,8 +299,10 @@ typedef enum StartBaseWorkerResult /* set up and utility functions */ +#if PG_VERSION_NUM < 190000 static void BaseWorkerSharedMemoryRequest(void); static void BaseWorkerSharedMemoryStartup(void); +#endif static void StartServerStarter(void); static void HandleSigterm(SIGNAL_ARGS); static void HandleSighup(SIGNAL_ARGS); @@ -381,9 +390,73 @@ static BaseWorkerControlData * BaseWorkerControl = NULL; static HTAB *DatabaseStarterHash = NULL; static HTAB *BaseWorkerHash = NULL; +#if PG_VERSION_NUM >= 190000 + +/* + * PG19 replaced the shmem_request_hook/shmem_startup_hook pair with a + * registered callback struct that drives the request and init phases. The + * struct is retained by pointer, so it must have static storage duration. + */ +static void BaseWorkerShmemRequest(void *arg); +static void BaseWorkerShmemInit(void *arg); + +static const ShmemCallbacks BaseWorkerShmemCallbacks = { + .request_fn = BaseWorkerShmemRequest, + .init_fn = BaseWorkerShmemInit, +}; + +static void +BaseWorkerShmemRequest(void *arg) +{ + HASHCTL info; + + /* the opts are deep-copied, but .name is kept by pointer, so use literals */ + ShmemRequestStruct(.name = "pg_extension_base server starter", + .size = sizeof(BaseWorkerControlData), + .ptr = (void **) &BaseWorkerControl); + + memset(&info, 0, sizeof(info)); + info.keysize = sizeof(Oid); + info.entrysize = sizeof(DatabaseStarterEntry); + info.hash = oid_hash; + ShmemRequestHash(.name = "pg_extension_base database starter hash", + .nelems = 2 * max_worker_processes, + .hash_info = info, + .hash_flags = HASH_ELEM | HASH_FUNCTION, + .ptr = &DatabaseStarterHash); + + memset(&info, 0, sizeof(info)); + info.keysize = sizeof(BaseWorkerKey); + info.entrysize = sizeof(BaseWorkerEntry); + info.hash = tag_hash; + ShmemRequestHash(.name = "pg_extension_base base worker hash", + .nelems = 2 * max_worker_processes, + .hash_info = info, + .hash_flags = HASH_ELEM | HASH_FUNCTION, + .ptr = &BaseWorkerHash); +} + +static void +BaseWorkerShmemInit(void *arg) +{ + /* + * The areas are allocated, zeroed, and *ptr is set before init_fn runs, + * which happens single-threaded, so we need neither AddinShmemInitLock + * nor a found check. LWLockNewTrancheId records the tranche name in + * shared memory, so forked backends report wait events without an + * attach_fn. + */ + BaseWorkerControl->lockTrancheName = "pg_extension_base server starter locks"; + BaseWorkerControl->trancheId = + LWLockNewTrancheId(BaseWorkerControl->lockTrancheName); + LWLockInitialize(&BaseWorkerControl->lock, BaseWorkerControl->trancheId); +} +#else + /* shared memory hooks */ static shmem_startup_hook_type PreviousSharedMemoryStartupHook = NULL; static shmem_request_hook_type PreviousSharedMemoryRequestHook = NULL; +#endif /* DDL hooks */ static ProcessUtility_hook_type PreviousProcessUtility = NULL; @@ -420,13 +493,15 @@ InitializeBaseWorkerLauncher(void) ProcessUtility_hook != NULL ? ProcessUtility_hook : standard_ProcessUtility; ProcessUtility_hook = BaseWorkerProcessUtility; - /* set up shared memory hooks */ - + /* set up shared memory */ +#if PG_VERSION_NUM >= 190000 + RegisterShmemCallbacks(&BaseWorkerShmemCallbacks); +#else PreviousSharedMemoryStartupHook = shmem_startup_hook; shmem_startup_hook = BaseWorkerSharedMemoryStartup; - PreviousSharedMemoryRequestHook = shmem_request_hook; shmem_request_hook = BaseWorkerSharedMemoryRequest; +#endif PreviousObjectAccessHook = object_access_hook; object_access_hook = BaseWorkerObjectAccessHook; @@ -437,6 +512,8 @@ InitializeBaseWorkerLauncher(void) } +#if PG_VERSION_NUM < 190000 + /* * BaseWorkerSharedMemorySize computes how much shared memory is required. */ @@ -493,6 +570,10 @@ BaseWorkerSharedMemoryStartup(void) /* * BaseWorkerSharedMemoryInit initializes the requested shared memory for the * worker hash and starts the server starter. + * + * PG19 replaced this hook-driven path with the RegisterShmemCallbacks + * mechanism (see BaseWorkerShmemRequest/BaseWorkerShmemInit above), so this + * function is PG18-and-earlier only. */ void BaseWorkerSharedMemoryInit(void) @@ -508,9 +589,8 @@ BaseWorkerSharedMemoryInit(void) if (!alreadyInitialized) { - BaseWorkerControl->trancheId = LWLockNewTrancheId(); BaseWorkerControl->lockTrancheName = "pg_extension_base server starter locks"; - + BaseWorkerControl->trancheId = LWLockNewTrancheId(); LWLockRegisterTranche(BaseWorkerControl->trancheId, BaseWorkerControl->lockTrancheName); @@ -542,6 +622,7 @@ BaseWorkerSharedMemoryInit(void) LWLockRelease(AddinShmemInitLock); } +#endif /* @@ -650,7 +731,11 @@ PgExtensionServerStarterMain(Datum arg) /* set up signal handlers */ pqsignal(SIGHUP, HandleSighup); pqsignal(SIGTERM, HandleSigterm); +#if PG_VERSION_NUM >= 190000 + pqsignal(SIGINT, PG_SIG_IGN); +#else pqsignal(SIGINT, SIG_IGN); +#endif before_shmem_exit(PgBaseExtensionServerStarterSharedMemoryExit, 0); @@ -1012,7 +1097,11 @@ PgExtensionBaseDatabaseStarterMain(Datum databaseIdDatum) /* Establish signal handlers before unblocking signals. */ pqsignal(SIGHUP, HandleSighup); +#if PG_VERSION_NUM >= 190000 + pqsignal(SIGINT, PG_SIG_IGN); +#else pqsignal(SIGINT, SIG_IGN); +#endif pqsignal(SIGTERM, HandleSigterm); /* Set our exit handler before any calls to proc_exit */ diff --git a/pg_extension_base/src/library_preloader.c b/pg_extension_base/src/library_preloader.c index fec185f62..9ab2c9651 100644 --- a/pg_extension_base/src/library_preloader.c +++ b/pg_extension_base/src/library_preloader.c @@ -36,6 +36,7 @@ #include "utils/builtins.h" #include "utils/conffiles.h" #include "utils/guc.h" +#include "utils/tuplestore.h" /* PreloadLibrary represents a library to preload */ typedef struct PreloadLibrary diff --git a/pg_extension_base/tests/pg_extension_base_test_common/Makefile b/pg_extension_base/tests/pg_extension_base_test_common/Makefile index eeb57acf6..542f91700 100644 --- a/pg_extension_base/tests/pg_extension_base_test_common/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_common/Makefile @@ -3,7 +3,7 @@ MODULE_big = pg_extension_base_test_common SOURCES := $(wildcard src/*.c) $(wildcard src/*/*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -g +PG_CPPFLAGS = -Iinclude -std=gnu11 -g override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) PG_CONFIG ?= pg_config diff --git a/pg_extension_base/tests/pg_extension_base_test_ext1/Makefile b/pg_extension_base/tests/pg_extension_base_test_ext1/Makefile index fa12390f3..b8d9fec19 100644 --- a/pg_extension_base/tests/pg_extension_base_test_ext1/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_ext1/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.0.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 +PG_CPPFLAGS = -Iinclude -std=gnu11 override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) PG_CONFIG ?= pg_config diff --git a/pg_extension_base/tests/pg_extension_base_test_ext2/Makefile b/pg_extension_base/tests/pg_extension_base_test_ext2/Makefile index bca71a828..76ab53839 100644 --- a/pg_extension_base/tests/pg_extension_base_test_ext2/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_ext2/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.0.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -I../pg_extension_base_test_common/include +PG_CPPFLAGS = -Iinclude -std=gnu11 -I../pg_extension_base_test_common/include override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) UNAME_S := $(shell uname -s) diff --git a/pg_extension_base/tests/pg_extension_base_test_ext3/Makefile b/pg_extension_base/tests/pg_extension_base_test_ext3/Makefile index 66773d9a3..ad00d8a96 100644 --- a/pg_extension_base/tests/pg_extension_base_test_ext3/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_ext3/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.0.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -I../pg_extension_base_test_common/include +PG_CPPFLAGS = -Iinclude -std=gnu11 -I../pg_extension_base_test_common/include override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) UNAME_S := $(shell uname -s) diff --git a/pg_extension_base/tests/pg_extension_base_test_hibernate/Makefile b/pg_extension_base/tests/pg_extension_base_test_hibernate/Makefile index 7dd1f94c2..3b19e1ba3 100644 --- a/pg_extension_base/tests/pg_extension_base_test_hibernate/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_hibernate/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.0.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -I../../include +PG_CPPFLAGS = -Iinclude -std=gnu11 -I../../include override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) UNAME_S := $(shell uname -s) diff --git a/pg_extension_base/tests/pg_extension_base_test_oneshot/Makefile b/pg_extension_base/tests/pg_extension_base_test_oneshot/Makefile index cb49b237c..7c3f39088 100644 --- a/pg_extension_base/tests/pg_extension_base_test_oneshot/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_oneshot/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.0.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -I../../include +PG_CPPFLAGS = -Iinclude -std=gnu11 -I../../include override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) UNAME_S := $(shell uname -s) diff --git a/pg_extension_base/tests/pg_extension_base_test_scheduler/Makefile b/pg_extension_base/tests/pg_extension_base_test_scheduler/Makefile index 62409f863..94e295e40 100644 --- a/pg_extension_base/tests/pg_extension_base_test_scheduler/Makefile +++ b/pg_extension_base/tests/pg_extension_base_test_scheduler/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.0.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -I../../include +PG_CPPFLAGS = -Iinclude -std=gnu11 -I../../include override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) UNAME_S := $(shell uname -s) diff --git a/pg_extension_updater/Makefile b/pg_extension_updater/Makefile index 9a3570f7b..69d39313e 100644 --- a/pg_extension_updater/Makefile +++ b/pg_extension_updater/Makefile @@ -6,7 +6,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--1.1.sql SOURCES := $(wildcard *.c) $(wildcard */*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -std=gnu99 -I../pg_extension_base/include +PG_CPPFLAGS = -Iinclude -std=gnu11 -I../pg_extension_base/include override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) UNAME_S := $(shell uname -s) diff --git a/pg_extension_updater/src/pg_extension_updater.c b/pg_extension_updater/src/pg_extension_updater.c index 758590ea4..6e0eb2b32 100644 --- a/pg_extension_updater/src/pg_extension_updater.c +++ b/pg_extension_updater/src/pg_extension_updater.c @@ -22,6 +22,7 @@ *------------------------------------------------------------------------- */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "miscadmin.h" diff --git a/pg_lake_benchmark/Makefile b/pg_lake_benchmark/Makefile index 472763663..610883dee 100644 --- a/pg_lake_benchmark/Makefile +++ b/pg_lake_benchmark/Makefile @@ -8,7 +8,7 @@ OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) SHLIB_LINK_INTERNAL = $(libpq) -PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -I../pg_extension_base/include -I../pg_lake_engine/include -std=gnu99 -g +PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -I../pg_extension_base/include -I../pg_lake_engine/include -std=gnu11 -g UNAME_S := $(shell uname -s) ifeq ($(UNAME_S),Darwin) diff --git a/pg_lake_benchmark/src/tpcds.c b/pg_lake_benchmark/src/tpcds.c index 06814338e..0255c4ae2 100644 --- a/pg_lake_benchmark/src/tpcds.c +++ b/pg_lake_benchmark/src/tpcds.c @@ -19,6 +19,7 @@ #include "fmgr.h" #include "funcapi.h" #include "utils/builtins.h" +#include "utils/tuplestore.h" #include "pg_lake/benchmark.h" #include "pg_lake/copy/copy_format.h" diff --git a/pg_lake_benchmark/src/tpch.c b/pg_lake_benchmark/src/tpch.c index af3a7ed88..735add109 100644 --- a/pg_lake_benchmark/src/tpch.c +++ b/pg_lake_benchmark/src/tpch.c @@ -19,6 +19,7 @@ #include "fmgr.h" #include "funcapi.h" #include "utils/builtins.h" +#include "utils/tuplestore.h" #include "pg_lake/benchmark.h" #include "pg_lake/copy/copy_format.h" diff --git a/pg_lake_copy/Makefile b/pg_lake_copy/Makefile index 2aa808db3..e472a1dab 100644 --- a/pg_lake_copy/Makefile +++ b/pg_lake_copy/Makefile @@ -7,7 +7,7 @@ SOURCES := $(wildcard src/*.c) $(wildcard src/*/*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) SHLIB_LINK = $(libpq) -PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -I../pg_extension_base/include -I../pg_lake_engine/include -I../pg_lake_iceberg/include -I../pg_lake_table/include -std=gnu99 -g -Wall -Wextra -Wno-unused-parameter -Werror +PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -I../pg_extension_base/include -I../pg_lake_engine/include -I../pg_lake_iceberg/include -I../pg_lake_table/include -std=gnu11 -g -Wall -Wextra -Wno-unused-parameter -Werror UNAME_S := $(shell uname -s) ifeq ($(UNAME_S),Darwin) diff --git a/pg_lake_copy/include/pg_lake/copy/copy.h b/pg_lake_copy/include/pg_lake/copy/copy.h index 9146b6f7d..4336ddbea 100644 --- a/pg_lake_copy/include/pg_lake/copy/copy.h +++ b/pg_lake_copy/include/pg_lake/copy/copy.h @@ -28,6 +28,36 @@ extern bool EnablePgLakeCopy; extern bool EnablePgLakeCopyJson; +/* + * JsonCopyMode controls who handles a JSON COPY. It is a hidden, test-only knob + * (see pg_lake_copy.json_copy_mode); normal users never set it and rely on the + * "auto" default. We follow the same rule as CSV: Postgres gets precedence + * whenever it natively supports the case, and pg_lake covers the rest. + * + * JSON_COPY_MODE_AUTO - surgical default: defer to Postgres only for the + * cases it supports (uncompressed COPY TO a local + * file/STDOUT); pg_lake handles remote targets, + * COPY FROM json, and compression. + * JSON_COPY_MODE_POSTGRES - always defer to Postgres (used by the upstream + * PostgreSQL regression suite so its expected + * output/error wording is reproduced verbatim). + * JSON_COPY_MODE_PGLAKE - always handle in pg_lake (used by pg_lake's own + * pytest suite to exercise the DuckDB JSON path). + */ +typedef enum JsonCopyModeType +{ + JSON_COPY_MODE_AUTO, + JSON_COPY_MODE_POSTGRES, + JSON_COPY_MODE_PGLAKE, +} JsonCopyModeType; + +/* GUC storage for an enum is an int (holds a JsonCopyModeType value) */ +extern int JsonCopyMode; + +/* allowed values for the pg_lake_copy.json_copy_mode enum GUC (defined in copy.c) */ +struct config_enum_entry; +extern const struct config_enum_entry json_copy_mode_options[]; + bool PgLakeCopyHandler(ProcessUtilityParams * params, void *arg); void ProcessPgLakeCopy(ParseState *pstate, PlannedStmt *plannedStmt, const char *queryString, uint64 *rowsProcessed); diff --git a/pg_lake_copy/src/copy/copy.c b/pg_lake_copy/src/copy/copy.c index aafa9b547..44881e393 100644 --- a/pg_lake_copy/src/copy/copy.c +++ b/pg_lake_copy/src/copy/copy.c @@ -25,6 +25,8 @@ #include "miscadmin.h" #include "libpq-fe.h" +#include "pg_extension_base/pg_compat.h" + #include "access/table.h" #include "access/tupdesc.h" #include "access/xact.h" @@ -80,6 +82,7 @@ #include "tcop/utility.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/portal.h" @@ -96,6 +99,9 @@ PgLakeCopyValidityCheckHookType PgLakeCopyValidityCheckHook = NULL; static bool IsPgLakeCopy(CopyStmt *copyStmt); +#if PG_VERSION_NUM >= 190000 +static bool PostgresSupportsJsonCopy(CopyStmt *copyStmt); +#endif static bool IsCopyFromStdin(CopyStmt *copyStmt); static bool IsCopyToStdout(CopyStmt *copyStmt); static void ProcessPgLakeCopyFrom(CopyStmt *copyStmt, ParseState *pstate, @@ -139,6 +145,15 @@ PG_FUNCTION_INFO_V1(pg_lake_last_copy_pushed_down_test); /* settings */ bool EnablePgLakeCopy = true; bool EnablePgLakeCopyJson = true; +int JsonCopyMode = JSON_COPY_MODE_AUTO; + +/* allowed values for the pg_lake_copy.json_copy_mode enum GUC */ +const struct config_enum_entry json_copy_mode_options[] = { + {"auto", JSON_COPY_MODE_AUTO, false}, + {"postgres", JSON_COPY_MODE_POSTGRES, false}, + {"pglake", JSON_COPY_MODE_PGLAKE, false}, + {NULL, 0, false} +}; /* * For COPY .. FROM, we convert incoming tuples into CSV format via a callback. @@ -239,7 +254,11 @@ IsPgLakeCopy(CopyStmt *copyStmt) { /* * Currently we only handle Parquet and JSON via STDIN/STDOUT, since - * PostgreSQL can already handle CSV. + * PostgreSQL can already handle CSV. This is the same principle + * applied to JSON below: give Postgres precedence whenever it + * natively supports the case, and only step in for what it cannot do + * (for CSV that is remote/lake targets, which already returned true + * above). * * We could probably add some useful features like CSV compression via * STDIN/STDIN, but the user experience of using different CSV parsers @@ -248,10 +267,91 @@ IsPgLakeCopy(CopyStmt *copyStmt) return false; } +#if PG_VERSION_NUM >= 190000 + + /* + * On PG19+ Postgres has a native JSON COPY for STDIN/STDOUT/local files + * (a supported URL/@stage already returned true above, so by here the + * target is local). Mirror the CSV rule: give Postgres precedence + * whenever it supports the case, and let pg_lake cover the rest. The + * hidden, test-only json_copy_mode GUC overrides this for the regression + * and pg_lake test suites. + */ + if (format == DATA_FORMAT_JSON) + { + switch (JsonCopyMode) + { + case JSON_COPY_MODE_PGLAKE: + /* always handle in pg_lake (exercise the DuckDB path) */ + return true; + case JSON_COPY_MODE_POSTGRES: + /* always defer to Postgres (upstream regression suite) */ + return false; + case JSON_COPY_MODE_AUTO: + default: + + /* + * Surgical default: defer to Postgres only for what it can do + * natively; pg_lake handles COPY FROM json and compression. + */ + if (PostgresSupportsJsonCopy(copyStmt)) + return false; + return true; + } + } +#endif + return true; } +#if PG_VERSION_NUM >= 190000 + +/* + * PostgresSupportsJsonCopy returns whether core PostgreSQL (PG19+) can natively + * handle this JSON COPY. Remote/lake targets are already claimed by pg_lake + * before this is reached, so the target here is always local/STDIN/STDOUT. + * + * Postgres supports only uncompressed COPY TO ... (FORMAT json): it rejects + * COPY FROM with the JSON format and has no compression option. We therefore + * keep COPY FROM json and compressed json on pg_lake, matching the CSV rule of + * giving Postgres precedence only where it actually supports the case. + */ +static bool +PostgresSupportsJsonCopy(CopyStmt *copyStmt) +{ + CopyDataCompression compression; + + /* + * IsPgLakeCopy claims every remote/lake target (s3://, gs://, az://, + * https://, @stage/...) for pg_lake before reaching here, so the target + * is always local/STDIN/STDOUT by now. Assert it so a future caller can't + * slip a URL past us and have us hand a remote path to core PostgreSQL. + */ + Assert(copyStmt->filename == NULL || + !IsSupportedURL(ResolveStageURL(copyStmt->filename))); + + if (copyStmt->is_from) + { + /* Postgres rejects COPY FROM ... (FORMAT json) */ + return false; + } + + /* compression from an explicit option, else inferred from the filename */ + compression = OptionsToCopyDataCompression(copyStmt->options); + if (compression == DATA_COMPRESSION_INVALID && copyStmt->filename != NULL) + { + compression = URLToCopyDataCompression(ResolveStageURL(copyStmt->filename)); + } + + /* Postgres has no compression; only plain output stays with Postgres */ + return compression == DATA_COMPRESSION_NONE || + compression == DATA_COMPRESSION_INVALID; +} + +#endif + + /* * IsCopyFromStdin returns whether the given COPY statement is of * the form COPY .. FROM STDIN. @@ -1039,6 +1139,17 @@ FindCopyToReadOptions(CopyDataFormat format, List *options) } } + else if (format == DATA_FORMAT_JSON) + { + /* + * FORCE_ARRAY is applied by the JSON writer (DuckDB ARRAY), not + * the intermediate CSV; accept it so it survives to + * ConvertCSVFileTo. + */ + if (strcmp(option->defname, "force_array") == 0) + continue; + } + /* * We currently do not propagate any other options. */ @@ -1176,13 +1287,22 @@ CheckCopyTableKind(CopyStmt *copyStmt, ParseState *pstate, Relation relation) (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot copy from sequence \"%s\"", RelationGetRelationName(relation)))); + + /* + * PG19 allows COPY TO directly, so on PG19+ we + * let a partitioned table fall through here (it is turned into a + * SELECT that descends into the partitions). On earlier versions we + * keep rejecting it to match core's per-version capability. + */ +#if PG_VERSION_NUM < 190000 else if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot copy from partitioned table \"%s\"", RelationGetRelationName(relation)), errhint("Try the COPY (SELECT ...) TO variant."))); - else +#endif + else if (relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot copy from non-table relation \"%s\"", @@ -1288,14 +1408,16 @@ CreateQueryForCopyToCommand(PlannedStmt *plannedStmt, Relation relation) /* * Build RangeVar for from clause, fully qualified based on the relation - * which we have opened and locked. Use "ONLY" so that COPY retrieves - * rows from only the target table not any inheritance children, the same - * as when RLS doesn't apply. + * which we have opened and locked. For ordinary tables (and foreign / + * iceberg tables) use "ONLY" so that COPY retrieves rows from just the + * target table and not any legacy INHERITS children, the same as when RLS + * doesn't apply. A partitioned parent holds no rows itself, so for + * PG19's COPY TO we must descend into its partitions. */ from = makeRangeVar(get_namespace_name(RelationGetNamespace(relation)), pstrdup(RelationGetRelationName(relation)), -1); - from->inh = false; /* apply ONLY */ + from->inh = (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); /* Build query */ select = makeNode(SelectStmt); @@ -1372,6 +1494,8 @@ BuildTupleDescriptorForRelation(Relation relation, List *attributeList) attributeNumber++; } + TupleDescFinalize(attributeDescriptor); + return attributeDescriptor; } @@ -1417,6 +1541,8 @@ RemoveDroppedColumnsFromTupleDesc(TupleDesc tableDescriptor) attributeNumber++; } + TupleDescFinalize(cleanTupleDesc); + return cleanTupleDesc; } diff --git a/pg_lake_copy/src/init.c b/pg_lake_copy/src/init.c index 1782ef878..5762e96ef 100644 --- a/pg_lake_copy/src/init.c +++ b/pg_lake_copy/src/init.c @@ -68,6 +68,33 @@ _PG_init(void) GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE, NULL, NULL, NULL); + /* + * Hidden, test-only knob controlling who handles a JSON COPY. We follow + * the same rule as CSV: Postgres gets precedence whenever it natively + * supports the case, and pg_lake covers the rest. + * + * auto - surgical default: defer to Postgres only for the cases it + * supports (PG19+ uncompressed COPY TO a local file/STDOUT); pg_lake + * handles remote targets, COPY FROM json, and compression. postgres - + * always defer to Postgres (used by the upstream PostgreSQL regression + * suite so its expected output/error wording is reproduced verbatim). + * pglake - always handle in pg_lake (used by pg_lake's own pytest suite + * to exercise the DuckDB JSON path). + * + * Before PG19 Postgres had no native JSON COPY, so pg_lake always handles + * it regardless of this setting. + */ + DefineCustomEnumVariable( + "pg_lake_copy.json_copy_mode", + gettext_noop("Controls whether JSON COPY is handled by Postgres or pg_lake (test-only)"), + NULL, + &JsonCopyMode, + JSON_COPY_MODE_AUTO, + json_copy_mode_options, + PGC_USERSET, + GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE, + NULL, NULL, NULL); + RegisterUtilityStatementHandler(CreateTableFromFileHandler, NULL); RegisterUtilityStatementHandler(PgLakeCopyHandler, NULL); } diff --git a/pg_lake_copy/src/test/types.c b/pg_lake_copy/src/test/types.c index 00ee41b6c..a560737ca 100644 --- a/pg_lake_copy/src/test/types.c +++ b/pg_lake_copy/src/test/types.c @@ -20,12 +20,15 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" +#include "pg_extension_base/pg_compat.h" #include "pg_lake/pgduck/type.h" #include "pg_lake/pgduck/parse_struct.h" #include "utils/builtins.h" +#include "utils/tuplestore.h" PG_FUNCTION_INFO_V1(duckdb_type_by_name); @@ -80,6 +83,7 @@ duckdb_parse_struct(PG_FUNCTION_ARGS) TupleDescInitEntry(tupdesc, (AttrNumber) 4, "isarray", BOOLOID, -1, 0); TupleDescInitEntry(tupdesc, (AttrNumber) 5, "level", INT4OID, -1, 0); + TupleDescFinalize(tupdesc); tupdesc = BlessTupleDesc(tupdesc); CompositeType *parsedType = ParseStructString(structString); diff --git a/pg_lake_copy/tests/pytests/test_json_copy.py b/pg_lake_copy/tests/pytests/test_json_copy.py index df0d4d927..108f380a0 100644 --- a/pg_lake_copy/tests/pytests/test_json_copy.py +++ b/pg_lake_copy/tests/pytests/test_json_copy.py @@ -2,10 +2,335 @@ import psycopg2 import time import duckdb +import json import math from utils_pytest import * +# This module is pg_lake's own JSON read/write suite. Under the production +# 'auto' default a plain local COPY TO json is served by Postgres (it gets +# precedence whenever it natively supports the case), which would silently move +# most of these assertions onto Postgres's writer. The internal tests here pin +# pg_lake-specific behaviour (DuckDB type inference, pg_lake error strings, +# option handling), so we default the whole module to 'pglake'. The committed +# SET is the per-connection baseline that each test's own rollback reverts to. +# +# The round-trip tests below override this per case via the `writer` parameter: +# they write the file with each writer in turn and always read it back through +# pg_lake (Postgres has no COPY FROM json), proving both producers emit JSON +# pg_lake can reconstruct losslessly. +JSON_WRITERS = ["postgres", "pglake"] + + +@pytest.fixture(autouse=True, scope="module") +def _default_pglake_json(pg_conn, superuser_conn, app_user): + for conn in (pg_conn, superuser_conn): + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", conn) + conn.commit() + # The writer-parametrized round-trips below also run with + # json_copy_mode=postgres, which routes COPY TO through + # PostgreSQL's own COPY -- that requires the pg_write_server_files role. + # Grant it to the (otherwise unprivileged) app_user so the same round-trip + # works for both producers. The role is module-scoped and dropped on + # teardown, so the grant never leaks. + run_command(f"GRANT pg_write_server_files TO {app_user}", superuser_conn) + superuser_conn.commit() + yield + + +def _set_json_writer(conn, writer): + """Select who writes the local JSON file for a round-trip test. Reverts to + the committed module default ('pglake') on the test's rollback.""" + run_command(f"SET pg_lake_copy.json_copy_mode TO '{writer}'", conn) + + +def _read_back_via_pglake(conn): + """The reader is always pg_lake; Postgres cannot COPY FROM json.""" + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", conn) + + +def _skip_pre_pg19(conn): + if get_pg_version_num(conn) < 190000: + pytest.skip("COPY ... (format json, force_array) requires PG19+") + + +# Multi-column, multi-row, ordered source covering a representative type spread +# (int, text with quotes + unicode, numeric, float, bool, NULL, date, +# timestamp) so a force_array round-trip exercises the whole JSON path. +_FA_SRC_DDL = """ +CREATE TABLE fa_src ( + id int, + name text, + score numeric(8,2), + ratio float8, + flag bool, + note text, + d date, + ts timestamp +); +INSERT INTO fa_src VALUES + (1, 'alice "the ace"', 1.50, 0.25, true, E'\\u0100\\U0001F642', '2024-01-01', '2024-01-01 15:00:00'), + (2, 'bob', 2.00, 1.5, false, NULL, '2023-06-15', '2023-06-15 08:30:00'), + (3, 'çelik', 3.25, NULL, NULL, 'plain', '2022-12-31', '2022-12-31 23:59:59'); +""" + +_FA_ROW_COUNT = 3 + + +def _fa_setup(conn): + run_command(_FA_SRC_DDL, conn) + run_command("CREATE TABLE fa_dst (LIKE fa_src)", conn) + + +def _fa_rows(conn, table): + return [dict(r) for r in run_query(f"SELECT * FROM {table} ORDER BY id", conn)] + + +@pytest.mark.parametrize("writer", JSON_WRITERS) +def test_force_array_roundtrip_local_file(pg_conn, tmp_path, writer): + """force_array writes a single JSON array to a local file and reads back. + + Parametrized over the producer: both Postgres and pg_lake must emit a JSON + array that pg_lake's COPY FROM reconstructs row-for-row. (app_user is + granted pg_write_server_files in the module fixture so the postgres writer's + COPY TO works.) + """ + _skip_pre_pg19(pg_conn) + + json_path = tmp_path / "fa_local.json" + + _fa_setup(pg_conn) + before = _fa_rows(pg_conn, "fa_src") + + _set_json_writer(pg_conn, writer) + run_command( + f"COPY fa_src TO '{json_path}' WITH (format json, force_array true)", pg_conn + ) + + content = json_path.read_bytes().strip() + assert content.startswith(b"["), f"expected JSON array, got: {content[:40]!r}" + assert content.endswith(b"]"), f"expected JSON array, got: {content[-40:]!r}" + parsed = json.loads(content) + assert isinstance(parsed, list) + assert len(parsed) == _FA_ROW_COUNT + + _read_back_via_pglake(pg_conn) + run_command(f"COPY fa_dst FROM '{json_path}' WITH (format json)", pg_conn) + after = _fa_rows(pg_conn, "fa_dst") + assert before == after + + pg_conn.rollback() + + +def test_force_array_roundtrip_s3(pg_conn, s3, tmp_path): + """force_array writes a single JSON array to an s3 lake target and reads back.""" + _skip_pre_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_force_array_roundtrip/fa_s3.json" + + _fa_setup(pg_conn) + before = _fa_rows(pg_conn, "fa_src") + + run_command(f"COPY fa_src TO '{url}' WITH (format json, force_array true)", pg_conn) + + content = read_s3_operations(s3, url, is_text=False).strip() + assert content.startswith(b"["), f"expected JSON array, got: {content[:40]!r}" + assert content.endswith(b"]"), f"expected JSON array, got: {content[-40:]!r}" + parsed = json.loads(content) + assert isinstance(parsed, list) + assert len(parsed) == _FA_ROW_COUNT + + run_command(f"COPY fa_dst FROM '{url}' WITH (format json)", pg_conn) + after = _fa_rows(pg_conn, "fa_dst") + assert before == after + + pg_conn.rollback() + + +@pytest.mark.parametrize("writer", JSON_WRITERS) +def test_no_force_array_ndjson_roundtrip(pg_conn, tmp_path, writer): + """Without force_array the same source stays NDJSON and still round-trips. + + Parametrized over the producer: both Postgres and pg_lake emit NDJSON (one + JSON object per line) that pg_lake reads back row-for-row. + """ + _skip_pre_pg19(pg_conn) + + json_path = tmp_path / "fa_ndjson.json" + + _fa_setup(pg_conn) + before = _fa_rows(pg_conn, "fa_src") + + _set_json_writer(pg_conn, writer) + run_command(f"COPY fa_src TO '{json_path}' WITH (format json)", pg_conn) + + content = json_path.read_bytes() + assert not content.lstrip().startswith(b"["), "expected NDJSON, got a JSON array" + lines = [ln for ln in content.splitlines() if ln.strip()] + assert len(lines) == _FA_ROW_COUNT + for ln in lines: + assert isinstance(json.loads(ln), dict) + + _read_back_via_pglake(pg_conn) + run_command(f"COPY fa_dst FROM '{json_path}' WITH (format json)", pg_conn) + after = _fa_rows(pg_conn, "fa_dst") + assert before == after + + pg_conn.rollback() + + +# --------------------------------------------------------------------------- +# PG19: COPY TO with JSON (plain + force_array). +# --------------------------------------------------------------------------- + +_PART_SRC_DDL = """ +DROP TABLE IF EXISTS pj_parent CASCADE; +DROP TABLE IF EXISTS pj_dst; +CREATE TABLE pj_parent (id int, name text, d date) PARTITION BY RANGE (id); +CREATE TABLE pj_parent_1 PARTITION OF pj_parent FOR VALUES FROM (0) TO (50); +CREATE TABLE pj_parent_2 PARTITION OF pj_parent FOR VALUES FROM (50) TO (100); +INSERT INTO pj_parent + SELECT g, 'name "' || g || '"', date '2024-01-01' + g + FROM generate_series(0, 99) g; +""" + +_PART_ROW_COUNT = 100 + + +def _pj_setup(conn): + run_command(_PART_SRC_DDL, conn) + run_command("CREATE TABLE pj_dst (LIKE pj_parent)", conn) + + +def _pj_rows(conn, table): + return [dict(r) for r in run_query(f"SELECT * FROM {table} ORDER BY id", conn)] + + +@pytest.mark.parametrize("writer", JSON_WRITERS) +def test_pg19_partitioned_json_roundtrip_local(pg_conn, tmp_path, writer): + """PG19: COPY TO (format json) to a local file then + COPY dst FROM round-trips every partition's rows. + + Parametrized over the producer: both Postgres (native COPY TO on the + partitioned parent) and pg_lake must emit NDJSON pg_lake reads back. + """ + _skip_pre_pg19(pg_conn) + + json_path = tmp_path / "pj_local.json" + _pj_setup(pg_conn) + before = _pj_rows(pg_conn, "pj_parent") + + _set_json_writer(pg_conn, writer) + run_command(f"COPY pj_parent TO '{json_path}' WITH (format json)", pg_conn) + + content = json_path.read_bytes() + assert not content.lstrip().startswith(b"["), "expected NDJSON, got a JSON array" + lines = [ln for ln in content.splitlines() if ln.strip()] + assert len(lines) == _PART_ROW_COUNT + + _read_back_via_pglake(pg_conn) + run_command(f"COPY pj_dst FROM '{json_path}' WITH (format json)", pg_conn) + after = _pj_rows(pg_conn, "pj_dst") + assert before == after + assert len(after) == _PART_ROW_COUNT + + pg_conn.rollback() + + +def test_pg19_partitioned_json_roundtrip_s3(pg_conn, s3, tmp_path): + """PG19: same as the local JSON round-trip but to an s3 lake target.""" + _skip_pre_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_pg19_partitioned_json/pj_s3.json" + _pj_setup(pg_conn) + before = _pj_rows(pg_conn, "pj_parent") + + run_command(f"COPY pj_parent TO '{url}' WITH (format json)", pg_conn) + + content = read_s3_operations(s3, url, is_text=False) + lines = [ln for ln in content.splitlines() if ln.strip()] + assert len(lines) == _PART_ROW_COUNT + + run_command(f"COPY pj_dst FROM '{url}' WITH (format json)", pg_conn) + after = _pj_rows(pg_conn, "pj_dst") + assert before == after + assert len(after) == _PART_ROW_COUNT + + pg_conn.rollback() + + +def _assert_single_json_array(content_bytes, expected_count): + """force_array output is a single JSON array. The exact framing differs by + producer (DuckDB uses tab indentation and `},\n\t{` separators, Postgres + uses spaces), so parse and count rather than byte-compare.""" + stripped = content_bytes.strip() + assert stripped.startswith(b"["), f"expected JSON array, got: {stripped[:40]!r}" + assert stripped.endswith(b"]"), f"expected JSON array, got: {stripped[-40:]!r}" + parsed = json.loads(stripped) + assert isinstance(parsed, list) + assert len(parsed) == expected_count + return parsed + + +@pytest.mark.parametrize("writer", JSON_WRITERS) +def test_pg19_partitioned_json_force_array_local(pg_conn, tmp_path, writer): + """PG19 nice story: COPY TO (format json, force_array + true) to a local file emits one JSON array spanning all partitions, then + COPY dst FROM reads it row-for-row. + + Parametrized over the producer: both Postgres and pg_lake must frame the + output as a single JSON array pg_lake reads back row-for-row. + """ + _skip_pre_pg19(pg_conn) + + json_path = tmp_path / "pj_fa_local.json" + _pj_setup(pg_conn) + before = _pj_rows(pg_conn, "pj_parent") + + _set_json_writer(pg_conn, writer) + run_command( + f"COPY pj_parent TO '{json_path}' WITH (format json, force_array true)", + pg_conn, + ) + + _assert_single_json_array(json_path.read_bytes(), _PART_ROW_COUNT) + + _read_back_via_pglake(pg_conn) + run_command(f"COPY pj_dst FROM '{json_path}' WITH (format json)", pg_conn) + after = _pj_rows(pg_conn, "pj_dst") + assert before == after + assert len(after) == _PART_ROW_COUNT + + pg_conn.rollback() + + +def test_pg19_partitioned_json_force_array_s3(pg_conn, s3, tmp_path): + """PG19 nice story (s3 variant): COPY TO an s3 lake + target with force_array emits a single JSON array, then COPY dst FROM reads + it back row-for-row.""" + _skip_pre_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_pg19_partitioned_json/pj_fa_s3.json" + _pj_setup(pg_conn) + before = _pj_rows(pg_conn, "pj_parent") + + run_command( + f"COPY pj_parent TO '{url}' WITH (format json, force_array true)", + pg_conn, + ) + + _assert_single_json_array( + read_s3_operations(s3, url, is_text=False), _PART_ROW_COUNT + ) + + run_command(f"COPY pj_dst FROM '{url}' WITH (format json)", pg_conn) + after = _pj_rows(pg_conn, "pj_dst") + assert before == after + assert len(after) == _PART_ROW_COUNT + + pg_conn.rollback() + + def test_types(pg_conn, duckdb_conn, tmp_path): run_command( """create schema if not exists lake_struct; diff --git a/pg_lake_copy/tests/pytests/test_json_copy_vs_core.py b/pg_lake_copy/tests/pytests/test_json_copy_vs_core.py new file mode 100644 index 000000000..0ea565b2c --- /dev/null +++ b/pg_lake_copy/tests/pytests/test_json_copy_vs_core.py @@ -0,0 +1,817 @@ +"""Parity tests for JSON COPY between pg_lake and core PostgreSQL (PG19+). + +pg_lake serves JSON COPY through DuckDB (table -> intermediate CSV -> DuckDB -> +JSON), whereas core PostgreSQL gained a native JSON COPY in PG19 that serializes +its types to JSON directly. These tests pin down, type by type and structure by +structure, where the two are byte-identical and where DuckDB's serialization +legitimately differs, so that future changes to either side are caught. + +The comparison toggles the hidden pg_lake_copy.json_copy_mode knob: 'pglake' +forces the COPY through DuckDB, 'postgres' forces the identical statement onto +core PostgreSQL. (The production default is 'auto', which would route a local +COPY TO json to Postgres, so these parity tests must force the producer +explicitly.) Single-row sources are used so output is one JSON line and row +ordering never matters. +""" + +import json + +import pytest +from utils_pytest import * + + +def _copy_to_stdout_json( + conn, source, tmp_path, fname, use_core, raise_error=True, force_array=False +): + """Capture the JSON produced by COPY TO STDOUT (format json). + + use_core selects the producer: True forces core PostgreSQL + (json_copy_mode=postgres), False forces pg_lake's DuckDB path + (json_copy_mode=pglake). force_array appends the FORCE_ARRAY option (single + JSON array). Returns the raw bytes on success, or the error string when + raise_error is False. + """ + run_command( + "SET pg_lake_copy.json_copy_mode TO " + + ("'postgres'" if use_core else "'pglake'"), + conn, + ) + options = "format 'json'" + if force_array: + options += ", force_array true" + path = tmp_path / fname + error = copy_to_file( + f"COPY {source} TO STDOUT WITH ({options})", path, conn, raise_error + ) + if error is not None: + return error + return path.read_bytes() + + +# Custom types needed by the structured cases below. Created inside the test +# transaction and rolled back, so they never leak between tests. +_STRUCT_SETUP = """ +CREATE TYPE vc_composite AS (x int, y text); +CREATE TYPE vc_enum AS ENUM ('a', 'b', 'c'); +CREATE DOMAIN vc_domain AS int CHECK (VALUE > 0); +""" + + +# Data types and structures for which pg_lake (DuckDB) and core emit byte-for-byte +# identical JSON. Each entry is a single-row SQL expression emitted as column "c". +_IDENTICAL_CASES = [ + ("int2", "1::int2"), + ("int4", "100000::int4"), + ("int8", "10000000000::int8"), + ("bool_true", "true"), + ("bool_false", "false"), + ("text", "'hello world'::text"), + ("text_quotes", "'a\"b'::text"), + ("text_backslash", "'a\\b'::text"), + ("text_newline", "E'a\\nb'::text"), + ("text_tab", "E'a\\tb'::text"), + ("text_carriage_return", "E'a\\rb'::text"), + ("text_backspace", "E'a\\bb'::text"), + ("text_formfeed", "E'a\\fb'::text"), + ("text_control_char", "E'a\\x01b'::text"), + ("text_quote_and_backslash", "E'a\"\\\\b'::text"), + ("text_unicode", "'Ā🙂'::text"), + ("empty_text", "''::text"), + ("float4", "3.5::float4"), + ("float8", "33333333.33444444::float8"), + ("numeric_mod", "99.99::numeric(4,2)"), + ("date", "'2024-01-01'::date"), + ("time", "'19:34:00'::time"), + ("interval", "'3 days'::interval"), + ("uuid", "'acd661ca-d18c-42e2-9c4e-61794318935e'::uuid"), + ("bytea", "'\\x0001'::bytea"), + ("inet", "'192.168.1.1'::inet"), + ("cidr", "'192.168.0.0/16'::cidr"), + ("bit", "B'1'"), + ("varbit", "B'0110'"), + ("money", "'$4.5'::money"), + ("json", '\'{"hello":"world"}\'::json'), + ("json_array", "'[1,2,3]'::json"), + ("int_array", "ARRAY[1,2,3]"), + ("text_array", "ARRAY['a','b']"), + ("array_with_null", "ARRAY[1,NULL,3]"), + ("null_value", "NULL::int"), + ("composite", "ROW(1, 'hi')::vc_composite"), + ("enum", "'b'::vc_enum"), + ("domain", "5::vc_domain"), + ("int4range", "'[1,5)'::int4range"), + ("numrange", "'[1.5,2.5]'::numrange"), + ("composite_array", "ARRAY[ROW(1,'a')::vc_composite, ROW(2,'b')::vc_composite]"), +] + + +# Types where DuckDB's JSON serialization differs from core's. We pin both the +# pg_lake and the core output so the divergence stays visible and intentional. +_DIFFERENT_CASES = [ + # Unconstrained numeric: DuckDB pads to the type's scale (trailing zeros). + ("numeric_small", "199.123::numeric", b'{"c":199.123000000}\n', b'{"c":199.123}\n'), + # numeric outside int64/double range: DuckDB emits it as a JSON string, + # core as a JSON number. + ( + "numeric_large", + "123456789012345678901234.99::numeric(39,2)", + b'{"c":"123456789012345678901234.99"}\n', + b'{"c":123456789012345678901234.99}\n', + ), + # Timestamp: DuckDB uses a space separator, core uses the ISO 8601 "T". + ( + "timestamp", + "'2024-01-01 15:00:00'::timestamp", + b'{"c":"2024-01-01 15:00:00"}\n', + b'{"c":"2024-01-01T15:00:00"}\n', + ), + # jsonb: DuckDB re-minifies (no space after colon), core preserves its + # canonical "key": value spacing. + ( + "jsonb", + '\'{"hello":"world"}\'::jsonb', + b'{"c":{"hello":"world"}}\n', + b'{"c":{"hello": "world"}}\n', + ), +] + + +# Every type/structure case, regardless of whether pg_lake's textual JSON +# matches core's. The representation differences above are cosmetic: a value +# written by pg_lake must always read back unchanged through pg_lake, so every +# case is also exercised as a write->read roundtrip below. +_ALL_TYPE_CASES = [(c[0], c[1]) for c in _IDENTICAL_CASES + _DIFFERENT_CASES] + + +def _skip_pre_pg19(conn): + if get_pg_version_num(conn) < 190000: + pytest.skip( + "native JSON COPY TO (and the pg_lake->core fallback) require PG19+" + ) + + +@pytest.mark.parametrize( + "case_id,expr", _IDENTICAL_CASES, ids=[c[0] for c in _IDENTICAL_CASES] +) +def test_json_to_stdout_matches_core(superuser_conn, case_id, expr, tmp_path): + _skip_pre_pg19(superuser_conn) + + try: + run_command(_STRUCT_SETUP, superuser_conn) + source = f"(SELECT {expr} AS c)" + lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "lake.json", False + ) + core = _copy_to_stdout_json(superuser_conn, source, tmp_path, "core.json", True) + finally: + superuser_conn.rollback() + + assert lake == core, f"{case_id}: pg_lake={lake!r} core={core!r}" + + +@pytest.mark.parametrize( + "case_id,expr,expected_lake,expected_core", + _DIFFERENT_CASES, + ids=[c[0] for c in _DIFFERENT_CASES], +) +def test_json_to_stdout_duckdb_differences( + superuser_conn, case_id, expr, expected_lake, expected_core, tmp_path +): + _skip_pre_pg19(superuser_conn) + + try: + source = f"(SELECT {expr} AS c)" + lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "lake.json", False + ) + core = _copy_to_stdout_json(superuser_conn, source, tmp_path, "core.json", True) + finally: + superuser_conn.rollback() + + assert lake == expected_lake, f"{case_id}: pg_lake output changed: {lake!r}" + assert core == expected_core, f"{case_id}: core output changed: {core!r}" + assert lake != core + + +def test_json_to_stdout_multidim_array_unsupported(superuser_conn, tmp_path): + """Multi-dimensional arrays cannot round-trip through pg_lake's CSV stage. + + Core handles them natively; pg_lake raises a clear DuckDB conversion error. + This documents the known gap so a future fix flips the assertion. + """ + _skip_pre_pg19(superuser_conn) + + try: + source = "(SELECT ARRAY[[1,2],[3,4]] AS c)" + lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "lake.json", False, raise_error=False + ) + finally: + superuser_conn.rollback() + + assert "Could not convert string" in lake + + try: + core = _copy_to_stdout_json(superuser_conn, source, tmp_path, "core.json", True) + finally: + superuser_conn.rollback() + + assert core == b'{"c":[[1,2],[3,4]]}\n' + + +def test_json_copy_from_roundtrip(pg_conn, tmp_path): + """pg_lake JSON write -> pg_lake JSON read preserves values across types. + + The representation differences above are irrelevant here: the JSON is parsed + straight back into the same PostgreSQL types, so values must be unchanged. + """ + json_path = tmp_path / "roundtrip.json" + + # Force pg_lake for both write and read so this exercises pg_lake's own JSON + # path (under the 'auto' default a local COPY TO json goes to Postgres). + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", pg_conn) + + run_command( + """ + CREATE TYPE rt_composite AS (x int, y text); + CREATE TABLE rt_src ( + c_int int, + c_text text, + c_numeric numeric, + c_bool bool, + c_json json, + c_jsonb jsonb, + c_int_array int[], + c_text_array text[], + c_ts timestamp, + c_composite rt_composite + ); + INSERT INTO rt_src VALUES + (1, 'hello', 199.123, true, '{"a":1}', '{"b":2}', + ARRAY[1,2,3], ARRAY['x','y'], '2024-01-01 15:00:00', ROW(7,'z')), + (2, NULL, NULL, false, NULL, '[1,2]', + ARRAY[NULL,5], ARRAY['a'], '1999-12-31 23:59:59', ROW(NULL,NULL)); + """, + pg_conn, + ) + + before = run_query("SELECT * FROM rt_src ORDER BY c_int", pg_conn) + + run_command( + f""" + COPY rt_src TO '{json_path}' WITH (format 'json'); + CREATE TABLE rt_dst (LIKE rt_src); + COPY rt_dst FROM '{json_path}' WITH (format 'json'); + """, + pg_conn, + ) + + after = run_query("SELECT * FROM rt_dst ORDER BY c_int", pg_conn) + + assert [dict(r) for r in before] == [dict(r) for r in after] + + pg_conn.rollback() + + +# Complex/nested types whose JSON serialization is non-trivial: composites +# (structs), arrays, arrays-of-composites, json/jsonb. The user-facing question +# is whether a file *written by core PostgreSQL* can be read back through +# pg_lake's DuckDB JSON path without losing structure. +_COMPLEX_RT_DDL = """ +CREATE TYPE crt_composite AS (x int, y text); +CREATE TABLE crt_src ( + c_int int, + c_int_array int[], + c_text_array text[], + c_array_with_null int[], + c_json json, + c_jsonb jsonb, + c_composite crt_composite, + c_composite_array crt_composite[] +); +INSERT INTO crt_src VALUES + (1, ARRAY[1,2,3], ARRAY['a','b'], ARRAY[1,NULL,3], + '{"a":1}', '{"b":2}', ROW(1,'hi'), + ARRAY[ROW(1,'a')::crt_composite, ROW(2,'b')::crt_composite]), + (2, ARRAY[]::int[], ARRAY['x"y','z\\\\w'], ARRAY[NULL,NULL]::int[], + NULL, '[1,2,3]', ROW(NULL,NULL), + ARRAY[ROW(NULL,'only')::crt_composite]); +""" + + +def _complex_rt_rows_for(conn, tbl): + # Compare on the text rendering so composite/array equality is exact and + # independent of how psycopg2 maps the nested types back to Python. + return run_query( + f""" + SELECT c_int, + c_int_array::text, + c_text_array::text, + c_array_with_null::text, + c_json::text, + c_jsonb::text, + c_composite::text, + c_composite_array::text + FROM {tbl} ORDER BY c_int + """, + conn, + ) + + +@pytest.mark.parametrize("writer", ["postgres", "pglake"], ids=["postgres", "pglake"]) +def test_json_complex_types_read_back_via_pg_lake(superuser_conn, writer, tmp_path): + """struct/array JSON written by either Postgres or pg_lake reads back via pg_lake. + + The writing of composites and arrays to JSON is the tricky part: Postgres + serializes them with its own type output functions, pg_lake routes through + DuckDB. This asserts that regardless of *who wrote* the JSON, pg_lake's JSON + COPY FROM (DuckDB) reconstructs the exact same nested values. + + A superuser connection is required because the Postgres writer path goes + through PostgreSQL's COPY TO , which needs pg_write_server_files. + """ + _skip_pre_pg19(superuser_conn) + + json_path = tmp_path / f"complex_{writer}.json" + try: + run_command(_COMPLEX_RT_DDL, superuser_conn) + before = _complex_rt_rows_for(superuser_conn, "crt_src") + + # Write the JSON with the requested producer. + run_command( + f"SET pg_lake_copy.json_copy_mode TO '{writer}'", + superuser_conn, + ) + run_command( + f"COPY crt_src TO '{json_path}' WITH (format 'json')", superuser_conn + ) + + # Always read back through pg_lake (DuckDB), never Postgres. + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", superuser_conn) + run_command("CREATE TABLE crt_dst (LIKE crt_src)", superuser_conn) + run_command( + f"COPY crt_dst FROM '{json_path}' WITH (format 'json')", superuser_conn + ) + + after = _complex_rt_rows_for(superuser_conn, "crt_dst") + finally: + superuser_conn.rollback() + + assert [dict(r) for r in before] == [ + dict(r) for r in after + ], f"{writer}-written JSON did not round-trip through pg_lake" + + +# pg_map map columns are the third "tricky" structured case alongside composites +# and arrays. A map renders as a JSON object ({"1":"a",...}); both Postgres and +# pg_lake emit the same shape. The extension/type are created inside the test +# transaction and rolled back, so they never leak between tests. +_MAP_RT_DDL = """ +CREATE EXTENSION IF NOT EXISTS pg_map; +SELECT map_type.create('int', 'text'); +CREATE TABLE map_src ( + c_int int, + c_map map_type.key_int_val_text +); +INSERT INTO map_src VALUES + (1, '{"(1,a)","(2,b)","(3,c)"}'), + (2, NULL), + (3, '{}'); +""" + + +def _map_rt_rows_for(conn, tbl): + # Compare on the text rendering so map equality is exact and independent of + # how psycopg2 maps the type back to Python. + return run_query(f"SELECT c_int, c_map::text FROM {tbl} ORDER BY c_int", conn) + + +def test_json_map_serialization_divergence(superuser_conn, tmp_path): + """pg_map maps: pg_lake's JSON round-trips, core's JSON does not. + + pg_lake (DuckDB) serializes a map column as a JSON object {"1":"a",...}, + which reads straight back into the map type. Core PostgreSQL serializes the + same map as its underlying array of {"key","val"} objects + ([{"key":1,"val":"a"},...]) -- a faithful but different shape that pg_lake's + DuckDB MAP reader cannot ingest (it expects an object). So unlike the + composite/array cases above, the producers are NOT interchangeable for maps; + this pins both the lossless pg_lake round-trip and the known core divergence. + + A superuser connection is required: map_type.create() and the Postgres + writer's COPY TO both need elevated privileges. + """ + _skip_pre_pg19(superuser_conn) + + lake_path = tmp_path / "map_pglake.json" + core_path = tmp_path / "map_core.json" + try: + run_command(_MAP_RT_DDL, superuser_conn) + before = _map_rt_rows_for(superuser_conn, "map_src") + + # pg_lake writer: object-shaped JSON that round-trips losslessly. + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", superuser_conn) + run_command( + f"COPY map_src TO '{lake_path}' WITH (format 'json')", superuser_conn + ) + run_command("CREATE TABLE map_dst (LIKE map_src)", superuser_conn) + run_command( + f"COPY map_dst FROM '{lake_path}' WITH (format 'json')", superuser_conn + ) + after = _map_rt_rows_for(superuser_conn, "map_dst") + + # core writer: array-of-{key,val} JSON. + run_command("SET pg_lake_copy.json_copy_mode TO 'postgres'", superuser_conn) + run_command( + f"COPY map_src TO '{core_path}' WITH (format 'json')", superuser_conn + ) + + # Reading core's array-shaped map JSON back through pg_lake fails: the + # DuckDB MAP reader expects an object, not an array. + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", superuser_conn) + run_command("TRUNCATE map_dst", superuser_conn) + core_read_err = run_command( + f"COPY map_dst FROM '{core_path}' WITH (format 'json')", + superuser_conn, + raise_error=False, + ) + finally: + superuser_conn.rollback() + + # pg_lake's own write -> read is lossless (covers NULL and empty maps too). + assert [dict(r) for r in before] == [dict(r) for r in after] + + # Pin the two serializations: pg_lake emits an object, core emits an array. + lake_first = json.loads(lake_path.read_text().splitlines()[0]) + core_first = json.loads(core_path.read_text().splitlines()[0]) + assert isinstance(lake_first["c_map"], dict), lake_first + assert isinstance(core_first["c_map"], list), core_first + + # Core's array-shaped map JSON is the known, non-round-trippable divergence. + assert core_read_err is not None + + +@pytest.mark.parametrize( + "case_id,expr", _ALL_TYPE_CASES, ids=[c[0] for c in _ALL_TYPE_CASES] +) +def test_json_roundtrip_per_type(pg_conn, case_id, expr, tmp_path): + """Every parity case must survive a pg_lake JSON write -> pg_lake read. + + This is the baseline guarantee for JSON COPY: the on-disk text may differ + from core's (see test_json_to_stdout_duckdb_differences), but reading the + value straight back into the same PostgreSQL type must reproduce it exactly. + CREATE TABLE AS captures the expression's own type so the destination column + matches the source. + """ + _skip_pre_pg19(pg_conn) + + path = tmp_path / "rt.json" + try: + # Force pg_lake for both write and read so this exercises pg_lake's own + # JSON path (under the 'auto' default a local COPY TO json goes to + # Postgres). + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", pg_conn) + run_command(_STRUCT_SETUP, pg_conn) + run_command(f"CREATE TABLE rtp_src AS SELECT {expr} AS c", pg_conn) + before = run_query("SELECT c::text AS c FROM rtp_src", pg_conn) + + run_command(f"COPY rtp_src TO '{path}' WITH (format 'json')", pg_conn) + run_command("CREATE TABLE rtp_dst (LIKE rtp_src)", pg_conn) + run_command(f"COPY rtp_dst FROM '{path}' WITH (format 'json')", pg_conn) + + after = run_query("SELECT c::text AS c FROM rtp_dst", pg_conn) + finally: + pg_conn.rollback() + + assert [dict(r) for r in before] == [ + dict(r) for r in after + ], f"{case_id}: before={before} after={after}" + + +def test_generated_column_handling(superuser_conn, tmp_path): + """Generated columns are omitted by COPY TO under json_copy_mode=postgres. + + Core PostgreSQL excludes GENERATED columns from COPY
TO so the + output round-trips through COPY FROM. With json_copy_mode=postgres pg_lake + defers to core, so the generated column is omitted exactly as Postgres does + -- this is the mode the upstream regression suite runs in, so we pin it. + + pg_lake's own (DuckDB) path historically *included* generated columns; that + divergence is a separately tracked, known issue + (https://github.com/Snowflake-Labs/pg_lake/issues/403) and is intentionally + not asserted here so this test stays stable regardless of that fix's state. + """ + _skip_pre_pg19(superuser_conn) + + try: + run_command( + """ + CREATE TABLE gen_tbl (a int, b int GENERATED ALWAYS AS (a * 2) STORED); + INSERT INTO gen_tbl (a) VALUES (5); + """, + superuser_conn, + ) + core = _copy_to_stdout_json( + superuser_conn, "gen_tbl", tmp_path, "core.json", True + ) + finally: + superuser_conn.rollback() + + assert core == b'{"a":5}\n' + + +# force_array array framing: DuckDB indents each element with a TAB, core with a +# single SPACE; otherwise the bytes are identical. JSON-escaped values never +# contain a raw newline+tab, so rewriting "\n\t" -> "\n " normalizes only the +# framing and lets us assert per-row byte parity precisely. +def _normalize_array_indent(data): + return data.replace(b"\n\t", b"\n ") + + +@pytest.mark.parametrize( + "case_id,expr", _IDENTICAL_CASES, ids=[c[0] for c in _IDENTICAL_CASES] +) +def test_force_array_to_stdout_matches_core(superuser_conn, case_id, expr, tmp_path): + """force_array maps to DuckDB ARRAY; output matches core modulo framing. + + pg_lake now accepts FORCE_ARRAY for JSON COPY TO and pushes it down to + DuckDB's ARRAY writer. The per-row JSON is byte-identical to core's; the only + difference is the array indentation character (DuckDB tab vs core space), + which we normalize before comparing. + """ + _skip_pre_pg19(superuser_conn) + + try: + run_command(_STRUCT_SETUP, superuser_conn) + source = f"(SELECT {expr} AS c)" + lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "lake.json", False, force_array=True + ) + core = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "core.json", True, force_array=True + ) + finally: + superuser_conn.rollback() + + assert lake.startswith(b"["), f"{case_id}: pg_lake not an array: {lake!r}" + assert ( + _normalize_array_indent(lake) == core + ), f"{case_id}: pg_lake={lake!r} core={core!r}" + + +def test_force_array_multirow_matches_core(superuser_conn, tmp_path): + """A multi-row ordered source with force_array yields the same JSON array as + core (the per-row objects are byte-identical; only the array framing -- comma + placement and indent char -- differs), and without force_array it is NDJSON. + + For multiple rows DuckDB writes `},\\n\\t{` between elements while core writes + `}\\n,{`, so the framing differs at the byte level; the documents (parsed + JSON) are equal and ordered, which is what matters. + """ + _skip_pre_pg19(superuser_conn) + + source = "(SELECT i FROM generate_series(1,3) i ORDER BY i)" + + try: + array_lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "lake.json", False, force_array=True + ) + array_core = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "core.json", True, force_array=True + ) + ndjson_lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "nd.json", False, force_array=False + ) + finally: + superuser_conn.rollback() + + assert array_lake.startswith(b"[") and array_core.startswith(b"[") + parsed_lake = json.loads(array_lake) + assert parsed_lake == json.loads(array_core) + assert parsed_lake == [{"i": 1}, {"i": 2}, {"i": 3}] + + # Without force_array the same source is newline-delimited JSON, not an array. + assert not ndjson_lake.lstrip().startswith(b"[") + nd_lines = [ln for ln in ndjson_lake.splitlines() if ln.strip()] + assert len(nd_lines) == 3 + for ln in nd_lines: + assert isinstance(json.loads(ln), dict) + + +def test_copy_from_json_stdin_behavior(pg_conn, tmp_path): + """COPY FROM (format json): pg_lake reads it, Postgres rejects it. + + Reading JSON is a pg_lake feature Postgres lacks (Postgres: "COPY FORMAT + JSON is not supported for COPY FROM"). Under the 'auto' default pg_lake + serves it (Postgres can't, so it never gets precedence here); with + json_copy_mode=postgres, Postgres's rejection applies, which is what the + PostgreSQL regression suite expects. + """ + _skip_pre_pg19(pg_conn) + + json_path = tmp_path / "rows.json" + with open(json_path, "w") as f: + f.write('{"x":1}\n{"x":2}\n') + + run_command("CREATE TABLE cf_tbl (x int)", pg_conn) + + # auto default: pg_lake reads the JSON (Postgres cannot) + run_command("SET pg_lake_copy.json_copy_mode TO 'auto'", pg_conn) + error = copy_from_file( + "COPY cf_tbl FROM STDIN WITH (format 'json')", + json_path, + pg_conn, + raise_error=False, + ) + assert error is None + assert run_query("SELECT count(*) AS c FROM cf_tbl", pg_conn)[0]["c"] == 2 + + # postgres mode: Postgres handles COPY FROM and rejects JSON + run_command("SET pg_lake_copy.json_copy_mode TO 'postgres'", pg_conn) + error = copy_from_file( + "COPY cf_tbl FROM STDIN WITH (format 'json')", + json_path, + pg_conn, + raise_error=False, + ) + assert error.startswith("ERROR: COPY FORMAT JSON is not supported for COPY FROM") + + pg_conn.rollback() + + +# JSON object keys are derived from the source's column names. pg_lake produces +# them via DuckDB (table -> CSV -> DuckDB), so its key derivation for unnamed / +# VALUES / duplicate columns must match core's, otherwise the JSON keys differ. +_COLUMN_NAMING_CASES = [ + # VALUES list with no column aliases -> column1, column2, ... + ("(VALUES (1, 'a'))", b'{"column1":1,"column2":"a"}\n'), + # a single unnamed SELECT expression -> ?column? + ("(SELECT 1)", b'{"?column?":1}\n'), + # mix of named and unnamed expressions + ("(SELECT 1 AS x, 2)", b'{"x":1,"?column?":2}\n'), +] + + +@pytest.mark.parametrize( + "source,expected", _COLUMN_NAMING_CASES, ids=[c[0] for c in _COLUMN_NAMING_CASES] +) +def test_json_column_naming_matches_core(superuser_conn, source, expected, tmp_path): + """JSON object keys for unnamed/VALUES/duplicate columns must match core.""" + _skip_pre_pg19(superuser_conn) + + try: + lake = _copy_to_stdout_json( + superuser_conn, source, tmp_path, "lake.json", False + ) + core = _copy_to_stdout_json(superuser_conn, source, tmp_path, "core.json", True) + finally: + superuser_conn.rollback() + + assert lake == expected, f"pg_lake key derivation changed: {lake!r}" + assert lake == core, f"pg_lake={lake!r} core={core!r}" + + +def test_json_bareword_format(superuser_conn, tmp_path): + """The format may be written as a bareword (format json) or a string + (format 'json'); pg_lake must accept both and produce identical output.""" + _skip_pre_pg19(superuser_conn) + + path_bare = tmp_path / "bare.json" + path_str = tmp_path / "str.json" + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", superuser_conn) + try: + copy_to_file( + "COPY (SELECT 1 AS c) TO STDOUT WITH (format json)", + path_bare, + superuser_conn, + ) + copy_to_file( + "COPY (SELECT 1 AS c) TO STDOUT WITH (format 'json')", + path_str, + superuser_conn, + ) + finally: + superuser_conn.rollback() + + assert path_bare.read_bytes() == b'{"c":1}\n' + assert path_bare.read_bytes() == path_str.read_bytes() + + +# CSV/text-only COPY options that Postgres rejects in JSON mode. pg_lake serves +# JSON COPY itself (json_copy_mode=pglake), so it must likewise refuse them -- +# never silently ignore an option that would change the output. With +# json_copy_mode=postgres, Postgres's own rejection applies. We only require that +# *both* sides reject; the wording differs (pg_lake: "invalid option ..."; +# Postgres: "cannot specify ... in JSON mode" / "requires CSV mode"). +_INCOMPATIBLE_JSON_OPTIONS = [ + ("header", "header true"), + ("delimiter", "delimiter ','"), + ("null", "null 'X'"), + ("quote", "quote '\"'"), + ("escape", "escape '\\'"), + ("force_quote", "force_quote (c)"), +] + + +@pytest.mark.parametrize( + "opt_name,opt_sql", + _INCOMPATIBLE_JSON_OPTIONS, + ids=[c[0] for c in _INCOMPATIBLE_JSON_OPTIONS], +) +def test_json_incompatible_options_rejected( + superuser_conn, opt_name, opt_sql, tmp_path +): + """A CSV-only option combined with format json must be rejected by both + pg_lake (json_copy_mode=pglake) and Postgres (json_copy_mode=postgres), not + silently accepted.""" + _skip_pre_pg19(superuser_conn) + + sql = f"COPY (SELECT 1 AS c) TO STDOUT WITH (format 'json', {opt_sql})" + + run_command("SET pg_lake_copy.json_copy_mode TO 'pglake'", superuser_conn) + lake_err = copy_to_file( + sql, tmp_path / "lake.json", superuser_conn, raise_error=False + ) + superuser_conn.rollback() + assert lake_err is not None, f"pg_lake silently accepted {opt_name} with json" + assert opt_name in lake_err.lower(), f"unexpected pg_lake error: {lake_err!r}" + + run_command("SET pg_lake_copy.json_copy_mode TO 'postgres'", superuser_conn) + core_err = copy_to_file( + sql, tmp_path / "core.json", superuser_conn, raise_error=False + ) + superuser_conn.rollback() + assert core_err is not None, f"Postgres silently accepted {opt_name} with json" + + +def test_force_array_rejected_for_from_and_csv(superuser_conn, tmp_path): + """force_array only applies to JSON COPY TO. + + It must be rejected for COPY FROM and for non-JSON formats by both pg_lake + (json_copy_mode=pglake) and Postgres (json_copy_mode=postgres), mirroring + Postgres's own restrictions (FORCE_ARRAY can only be used with JSON mode and + not with COPY FROM). Each iteration rolls back since the failed statement + aborts the transaction. + """ + _skip_pre_pg19(superuser_conn) + + # (format csv): FORCE_ARRAY requires JSON mode -> rejected either way + csv_copy = "COPY (SELECT 1 AS c) TO STDOUT WITH (format 'csv', force_array true)" + for mode in ("pglake", "postgres"): + run_command( + f"SET pg_lake_copy.json_copy_mode TO '{mode}'", + superuser_conn, + ) + err = copy_to_file( + csv_copy, tmp_path / "csv.out", superuser_conn, raise_error=False + ) + superuser_conn.rollback() + assert err is not None, f"force_array+csv accepted (mode={mode})" + + # COPY FROM (format json): FORCE_ARRAY cannot be combined with COPY FROM. + # Commit the table so it survives the per-iteration rollback. + json_path = tmp_path / "rows.json" + with open(json_path, "w") as f: + f.write('{"x":1}\n') + + run_command("CREATE TABLE fa_from (x int)", superuser_conn) + superuser_conn.commit() + try: + for mode in ("pglake", "postgres"): + run_command( + f"SET pg_lake_copy.json_copy_mode TO '{mode}'", + superuser_conn, + ) + err = copy_from_file( + "COPY fa_from FROM STDIN WITH (format 'json', force_array true)", + json_path, + superuser_conn, + raise_error=False, + ) + superuser_conn.rollback() + assert err is not None, f"force_array+COPY FROM accepted (mode={mode})" + finally: + run_command("DROP TABLE fa_from", superuser_conn) + superuser_conn.commit() + + +def test_lake_target_ignores_json_copy_mode(pg_conn): + """Lake targets are always served by pg_lake, even in json_copy_mode=postgres. + + A supported URL is claimed by pg_lake before the JSON deferral is reached, + so an option pg_lake does not support still produces pg_lake's own error + (Postgres cannot write to s3 anyway). + """ + url = f"s3://{TEST_BUCKET}/test_lake_knob/data.json" + run_command("CREATE TABLE lk_s3 (a int); INSERT INTO lk_s3 VALUES (1)", pg_conn) + run_command("SET pg_lake_copy.json_copy_mode TO 'postgres'", pg_conn) + + # quote is a CSV-only option pg_lake rejects for JSON; reaching pg_lake's + # error (not Postgres's) proves the lake target stayed on pg_lake's path. + error = run_command( + f"COPY lk_s3 TO '{url}' WITH (format 'json', quote '|')", + pg_conn, + raise_error=False, + ) + assert 'pg_lake_copy: invalid option "quote"' in error + + pg_conn.rollback() diff --git a/pg_lake_copy/tests/pytests/test_parquet_copy.py b/pg_lake_copy/tests/pytests/test_parquet_copy.py index f5521535e..958aab769 100644 --- a/pg_lake_copy/tests/pytests/test_parquet_copy.py +++ b/pg_lake_copy/tests/pytests/test_parquet_copy.py @@ -5,6 +5,13 @@ import math from utils_pytest import * +PG19 = 190000 + + +def _skip_pre_pg19(conn): + if get_pg_version_num(conn) < PG19: + pytest.skip("COPY TO requires PG19+") + def test_types(pg_conn, duckdb_conn, superuser_conn, tmp_path, app_user): parquet_path = tmp_path / "test_types.parquet" @@ -706,15 +713,288 @@ def test_partitioned(pg_conn, duckdb_conn, tmp_path): assert result[0]["count"] == 4 assert result[0]["distinct"] == 2 - result = run_command( + if get_pg_version_num(pg_conn) >= 190000: + # PG19 allows COPY TO directly: it must descend + # into the partitions and stream every row out, then round-trip back. + direct_path = tmp_path / "test_direct.parquet" + run_command( + f"COPY test_partitioned TO '{direct_path}' WITH (format 'parquet')", + pg_conn, + ) + run_command( + "CREATE TABLE test_partitioned_rt (t date, data text)", + pg_conn, + ) + run_command( + f"COPY test_partitioned_rt FROM '{direct_path}' WITH (format 'parquet')", + pg_conn, + ) + rt = run_query( + "SELECT count(*) AS count, count(distinct data) AS distinct " + "FROM test_partitioned_rt", + pg_conn, + ) + assert rt[0]["count"] == 4 + assert rt[0]["distinct"] == 2 + else: + result = run_command( + f""" + COPY test_partitioned TO '{parquet_path}' WITH (format 'parquet'); + """, + pg_conn, + raise_error=False, + ) + + assert "cannot copy from partitioned table" in result + + pg_conn.rollback() + + +def test_pg19_copy_to_all_heap_partitioned(pg_conn, tmp_path): + """PG19: COPY TO with all-heap partitions streams + every partition's rows out, and round-trips into a plain table.""" + _skip_pre_pg19(pg_conn) + + out_path = tmp_path / "all_heap.parquet" + run_command( + """ + DROP TABLE IF EXISTS p19_heap_parent CASCADE; + DROP TABLE IF EXISTS p19_heap_dst; + CREATE TABLE p19_heap_parent (id int, v text) PARTITION BY RANGE (id); + CREATE TABLE p19_heap_p1 PARTITION OF p19_heap_parent + FOR VALUES FROM (0) TO (100); + CREATE TABLE p19_heap_p2 PARTITION OF p19_heap_parent + FOR VALUES FROM (100) TO (200); + INSERT INTO p19_heap_parent + SELECT g, 'v' || g FROM generate_series(0, 199) g; + """, + pg_conn, + ) + + run_command( + f"COPY p19_heap_parent TO '{out_path}' WITH (format 'parquet')", + pg_conn, + ) + + run_command("CREATE TABLE p19_heap_dst (id int, v text)", pg_conn) + run_command( + f"COPY p19_heap_dst FROM '{out_path}' WITH (format 'parquet')", + pg_conn, + ) + + src = run_query("SELECT id, v FROM p19_heap_parent ORDER BY id", pg_conn) + dst = run_query("SELECT id, v FROM p19_heap_dst ORDER BY id", pg_conn) + assert [dict(r) for r in src] == [dict(r) for r in dst] + assert len(dst) == 200 + + pg_conn.rollback() + + +def test_pg19_copy_to_mixed_partition_tree(pg_conn, s3, extension, tmp_path): + """PG19 HEADLINE: COPY TO where leaves are a mix of a + heap partition and a pg_lake foreign-table partition (parquet on s3). The + inh=true descent must read the foreign child through the FDW so ALL rows + appear in the output.""" + _skip_pre_pg19(pg_conn) + + ft_url = f"s3://{TEST_BUCKET}/test_pg19_copy_mixed/ft_child.parquet" + out_url = f"s3://{TEST_BUCKET}/test_pg19_copy_mixed/parent_out.parquet" + + # Write the parquet backing the foreign-table partition (ids 100..199). + run_command( f""" - COPY test_partitioned TO '{parquet_path}' WITH (format 'parquet'); - """, + COPY (SELECT g AS id, 'v' || g AS v + FROM generate_series(100, 199) g) + TO '{ft_url}' WITH (format 'parquet') + """, + pg_conn, + ) + pg_conn.commit() + + run_command( + """ + DROP TABLE IF EXISTS p19_mixed_parent CASCADE; + DROP TABLE IF EXISTS p19_mixed_dst; + CREATE TABLE p19_mixed_parent (id int, v text) PARTITION BY RANGE (id); + CREATE TABLE p19_mixed_heap PARTITION OF p19_mixed_parent + FOR VALUES FROM (0) TO (100); + INSERT INTO p19_mixed_heap + SELECT g, 'v' || g FROM generate_series(0, 99) g; + """, + pg_conn, + ) + run_command( + f""" + CREATE FOREIGN TABLE p19_mixed_ft + PARTITION OF p19_mixed_parent FOR VALUES FROM (100) TO (200) + SERVER pg_lake OPTIONS (format 'parquet', path '{ft_url}') + """, + pg_conn, + ) + pg_conn.commit() + + run_command( + f"COPY p19_mixed_parent TO '{out_url}' WITH (format 'parquet')", + pg_conn, + ) + pg_conn.commit() + + run_command("CREATE TABLE p19_mixed_dst (id int, v text)", pg_conn) + run_command( + f"COPY p19_mixed_dst FROM '{out_url}' WITH (format 'parquet')", + pg_conn, + ) + + src = run_query("SELECT id, v FROM p19_mixed_parent ORDER BY id", pg_conn) + dst = run_query("SELECT id, v FROM p19_mixed_dst ORDER BY id", pg_conn) + assert [dict(r) for r in src] == [dict(r) for r in dst] + # 100 heap rows + 100 foreign-table rows must all be present. + assert len(dst) == 200 + assert dst[0]["id"] == 0 and dst[-1]["id"] == 199 + + run_command("DROP TABLE IF EXISTS p19_mixed_parent CASCADE", pg_conn) + run_command("DROP TABLE IF EXISTS p19_mixed_dst", pg_conn) + pg_conn.commit() + + +def test_pg19_copy_to_mixed_tree_with_iceberg_child( + pg_conn, s3, with_default_location, tmp_path +): + """PG19: like the mixed-tree test but one leaf is an iceberg child created + via PARTITION OF ... USING iceberg. Proves the descent reads iceberg + partitions through the FDW too.""" + _skip_pre_pg19(pg_conn) + + out_url = f"s3://{TEST_BUCKET}/test_pg19_copy_iceberg/parent_out.parquet" + + run_command( + """ + DROP TABLE IF EXISTS p19_ice_parent CASCADE; + DROP TABLE IF EXISTS p19_ice_dst; + CREATE TABLE p19_ice_parent (id int, v text) PARTITION BY RANGE (id); + CREATE TABLE p19_ice_heap PARTITION OF p19_ice_parent + FOR VALUES FROM (0) TO (100); + """, + pg_conn, + ) + run_command( + """ + CREATE TABLE p19_ice_child PARTITION OF p19_ice_parent + FOR VALUES FROM (100) TO (200) USING iceberg + """, + pg_conn, + ) + run_command( + """ + INSERT INTO p19_ice_parent + SELECT g, 'v' || g FROM generate_series(0, 199) g; + """, + pg_conn, + ) + pg_conn.commit() + + run_command( + f"COPY p19_ice_parent TO '{out_url}' WITH (format 'parquet')", + pg_conn, + ) + pg_conn.commit() + + run_command("CREATE TABLE p19_ice_dst (id int, v text)", pg_conn) + run_command( + f"COPY p19_ice_dst FROM '{out_url}' WITH (format 'parquet')", + pg_conn, + ) + + src = run_query("SELECT id, v FROM p19_ice_parent ORDER BY id", pg_conn) + dst = run_query("SELECT id, v FROM p19_ice_dst ORDER BY id", pg_conn) + assert [dict(r) for r in src] == [dict(r) for r in dst] + assert len(dst) == 200 + + run_command("DROP TABLE IF EXISTS p19_ice_parent CASCADE", pg_conn) + run_command("DROP TABLE IF EXISTS p19_ice_dst", pg_conn) + pg_conn.commit() + + +def test_pg19_copy_to_column_subset(pg_conn, tmp_path): + """PG19: COPY parent (col_a, col_b) TO ... must descend into partitions and + project only the requested columns.""" + _skip_pre_pg19(pg_conn) + + out_path = tmp_path / "subset.parquet" + run_command( + """ + DROP TABLE IF EXISTS p19_sub_parent CASCADE; + DROP TABLE IF EXISTS p19_sub_dst; + CREATE TABLE p19_sub_parent (id int, a text, b int) + PARTITION BY RANGE (id); + CREATE TABLE p19_sub_p1 PARTITION OF p19_sub_parent + FOR VALUES FROM (0) TO (100); + CREATE TABLE p19_sub_p2 PARTITION OF p19_sub_parent + FOR VALUES FROM (100) TO (200); + INSERT INTO p19_sub_parent + SELECT g, 'a' || g, g * 10 FROM generate_series(0, 199) g; + """, + pg_conn, + ) + + run_command( + f"COPY p19_sub_parent (id, b) TO '{out_path}' WITH (format 'parquet')", + pg_conn, + ) + + run_command("CREATE TABLE p19_sub_dst (id int, b int)", pg_conn) + run_command( + f"COPY p19_sub_dst FROM '{out_path}' WITH (format 'parquet')", + pg_conn, + ) + + src = run_query("SELECT id, b FROM p19_sub_parent ORDER BY id", pg_conn) + dst = run_query("SELECT id, b FROM p19_sub_dst ORDER BY id", pg_conn) + assert [dict(r) for r in src] == [dict(r) for r in dst] + assert len(dst) == 200 + + pg_conn.rollback() + + +def test_pg19_copy_only_parent_yields_zero_rows(pg_conn, tmp_path): + """PG19: COPY ONLY TO ... honors ONLY semantics -- the + parent itself holds no rows, so the output is empty even though the + partitions are populated.""" + _skip_pre_pg19(pg_conn) + + out_path = tmp_path / "only_parent.parquet" + run_command( + """ + DROP TABLE IF EXISTS p19_only_parent CASCADE; + DROP TABLE IF EXISTS p19_only_dst; + CREATE TABLE p19_only_parent (id int, v text) PARTITION BY RANGE (id); + CREATE TABLE p19_only_p1 PARTITION OF p19_only_parent + FOR VALUES FROM (0) TO (100); + INSERT INTO p19_only_parent + SELECT g, 'v' || g FROM generate_series(0, 99) g; + """, + pg_conn, + ) + + err = run_command( + f"COPY ONLY p19_only_parent TO '{out_path}' WITH (format 'parquet')", pg_conn, raise_error=False, ) - assert "cannot copy from partitioned table" in result + if err is not None: + # COPY ONLY on a partitioned parent may be rejected by core/pg_lake + # before reaching our path; in that case ONLY semantics are moot. + pg_conn.rollback() + pytest.skip(f"COPY ONLY on partitioned parent not accepted: {err!r}") + + run_command("CREATE TABLE p19_only_dst (id int, v text)", pg_conn) + run_command( + f"COPY p19_only_dst FROM '{out_path}' WITH (format 'parquet')", + pg_conn, + ) + dst = run_query("SELECT count(*) AS n FROM p19_only_dst", pg_conn) + assert int(dst[0]["n"]) == 0 pg_conn.rollback() diff --git a/pg_lake_copy/tests/pytests/test_pg19_copy_options.py b/pg_lake_copy/tests/pytests/test_pg19_copy_options.py new file mode 100644 index 000000000..bc42b7dc4 --- /dev/null +++ b/pg_lake_copy/tests/pytests/test_pg19_copy_options.py @@ -0,0 +1,122 @@ +"""PG19 COPY option behavior on pg_lake data-lake targets. + +PostgreSQL 19 extended COPY with multi-line ``HEADER n`` skipping and +``ON_ERROR SET_NULL``, and tightened COPY FROM ... WHERE to disallow system +columns. pg_lake serves COPY to/from data-lake URLs through DuckDB and parses +the options itself, so these tests pin how pg_lake currently behaves for each +new option on a lake target: either it is enforced (system columns) or it is +cleanly rejected (a documented gap), never silently applied incorrectly. +""" + +import pytest +from utils_pytest import * + +PG19 = 190000 + + +def _skip_pre_pg19(conn): + if get_pg_version_num(conn) < PG19: + pytest.skip("PG19 COPY option behavior") + + +def _write_lake_csv(conn, url, ddl, insert): + run_command("DROP TABLE IF EXISTS copt_src", conn) + run_command("DROP TABLE IF EXISTS copt_dst", conn) + run_command(ddl, conn) + run_command(insert, conn) + run_command(f"COPY copt_src TO '{url}' WITH (format csv, header true)", conn) + conn.commit() + + +def test_multiline_header_rejected_on_lake_copy(pg_conn): + """PG19 ``HEADER n`` (n > 1, skip multiple header lines) is not yet + implemented for pg_lake lake COPY; it must be rejected cleanly (not silently + treated as a single header) for both COPY FROM and COPY TO. Documents the + gap so a future implementation flips these assertions. + """ + _skip_pre_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_pg19_copy_options/header.csv" + _write_lake_csv( + pg_conn, + url, + "CREATE TABLE copt_src (a int, b text)", + "INSERT INTO copt_src VALUES (1,'x'),(2,'y')", + ) + run_command("CREATE TABLE copt_dst (a int, b text)", pg_conn) + pg_conn.commit() + + from_err = run_command( + f"COPY copt_dst FROM '{url}' WITH (format csv, header 2)", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert ( + from_err and "header" in from_err.lower() + ), f"HEADER 2 on lake COPY FROM should be rejected cleanly, got: {from_err!r}" + + to_err = run_command( + f"COPY copt_src TO '{url}' WITH (format csv, header 2)", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert ( + to_err and "header" in to_err.lower() + ), f"HEADER 2 on lake COPY TO should be rejected cleanly, got: {to_err!r}" + + +def test_on_error_set_null_rejected_on_lake_copy(pg_conn): + """PG19 ``ON_ERROR SET_NULL`` (and ON_ERROR generally) is not implemented for + pg_lake lake COPY FROM; it must be rejected cleanly rather than silently + ignored, which would let malformed input through. Documents the gap. + """ + _skip_pre_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_pg19_copy_options/on_error.csv" + _write_lake_csv( + pg_conn, + url, + "CREATE TABLE copt_src (a text, b text)", + "INSERT INTO copt_src VALUES ('notanint','x'),('2','y')", + ) + run_command("CREATE TABLE copt_dst (a int, b text)", pg_conn) + pg_conn.commit() + + err = run_command( + f"COPY copt_dst FROM '{url}' WITH (format csv, header true, on_error set_null)", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert ( + err and "on_error" in err.lower() + ), f"ON_ERROR SET_NULL on lake COPY FROM should be rejected cleanly, got: {err!r}" + + +def test_system_column_in_copy_from_where_rejected(pg_conn): + """PG19 disallows system columns in COPY FROM ... WHERE. A COPY FROM a lake + file whose WHERE references a system column must be rejected (incoming rows + have no system columns), not silently evaluated.""" + _skip_pre_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_pg19_copy_options/syscol.csv" + _write_lake_csv( + pg_conn, + url, + "CREATE TABLE copt_src (a int, b text)", + "INSERT INTO copt_src VALUES (1,'x'),(2,'y')", + ) + run_command("CREATE TABLE copt_dst (a int, b text)", pg_conn) + pg_conn.commit() + + err = run_command( + f"COPY copt_dst FROM '{url}' WITH (format csv, header true) WHERE xmin <> 0", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert ( + err and "system column" in err.lower() + ), f"system column in COPY FROM WHERE should be rejected, got: {err!r}" diff --git a/pg_lake_engine/Makefile b/pg_lake_engine/Makefile index d5497a02a..2f2842c4c 100644 --- a/pg_lake_engine/Makefile +++ b/pg_lake_engine/Makefile @@ -9,7 +9,7 @@ SHLIB_LINK_INTERNAL = $(libpq) PG_LAKE_DELTA_SUPPORT ?= 0 -PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -I../pg_extension_base/include -std=gnu99 -g -DPG_LAKE_DELTA_SUPPORT=$(PG_LAKE_DELTA_SUPPORT) +PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -I../pg_extension_base/include -std=gnu11 -g -DPG_LAKE_DELTA_SUPPORT=$(PG_LAKE_DELTA_SUPPORT) UNAME_S := $(shell uname -s) ifeq ($(UNAME_S),Darwin) diff --git a/pg_lake_engine/include/pg_lake/csv/csv_options.h b/pg_lake_engine/include/pg_lake/csv/csv_options.h index d26afe1bb..dd16f2e48 100644 --- a/pg_lake_engine/include/pg_lake/csv/csv_options.h +++ b/pg_lake_engine/include/pg_lake/csv/csv_options.h @@ -20,6 +20,15 @@ #include "commands/copy.h" #include "nodes/pg_list.h" +#if PG_VERSION_NUM >= 190000 +/* + * PG19 dropped the CopyHeaderChoice enum and stores the header choice as a + * plain int holding COPY_HEADER_FALSE / COPY_HEADER_TRUE / COPY_HEADER_MATCH. + * Reintroduce the typedef here so we keep a stable signature. + */ +typedef int CopyHeaderChoice; +#endif + extern PGDLLEXPORT List *InternalCSVOptions(bool includeHeader); extern PGDLLEXPORT List *NormalizedExternalCSVOptions(List *inputOptions); extern PGDLLEXPORT CopyHeaderChoice GetCopyHeaderChoice(DefElem *def, bool is_from); diff --git a/pg_lake_engine/include/pg_lake/pgduck/serialize.h b/pg_lake_engine/include/pg_lake/pgduck/serialize.h index d76f4b0be..7c7cf7ae8 100644 --- a/pg_lake_engine/include/pg_lake/pgduck/serialize.h +++ b/pg_lake_engine/include/pg_lake/pgduck/serialize.h @@ -18,6 +18,7 @@ #pragma once #include "postgres.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "pg_lake/parquet/field.h" diff --git a/pg_lake_engine/src/cleanup/deletion_queue.c b/pg_lake_engine/src/cleanup/deletion_queue.c index a6d00640d..efbc258cd 100644 --- a/pg_lake_engine/src/cleanup/deletion_queue.c +++ b/pg_lake_engine/src/cleanup/deletion_queue.c @@ -19,6 +19,7 @@ * Functions for cleaning up orphaned files. */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "funcapi.h" #include "miscadmin.h" @@ -30,6 +31,7 @@ #include "pg_lake/util/string_utils.h" #include "datatype/timestamp.h" #include "storage/procarray.h" +#include "utils/tuplestore.h" #define DELETION_QUEUE_TABLE "lake_engine.deletion_queue" diff --git a/pg_lake_engine/src/cleanup/in_progress_files.c b/pg_lake_engine/src/cleanup/in_progress_files.c index 67b55644f..9d858ffce 100644 --- a/pg_lake_engine/src/cleanup/in_progress_files.c +++ b/pg_lake_engine/src/cleanup/in_progress_files.c @@ -24,6 +24,7 @@ * via VACUUM. */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "funcapi.h" #include "miscadmin.h" @@ -42,11 +43,13 @@ #include "pg_lake/util/plan_cache.h" #include "pg_lake/util/string_utils.h" #include "datatype/timestamp.h" +#include "storage/lock.h" #include "storage/procarray.h" #include "utils/fmgroids.h" #include "utils/memutils.h" #include "utils/lsyscache.h" #include "utils/snapmgr.h" +#include "utils/tuplestore.h" #define OPERATION_ID_SEQUENCE "operationid_seq" diff --git a/pg_lake_engine/src/csv/csv_writer.c b/pg_lake_engine/src/csv/csv_writer.c index 3b91870fe..4974f9d26 100644 --- a/pg_lake_engine/src/csv/csv_writer.c +++ b/pg_lake_engine/src/csv/csv_writer.c @@ -42,7 +42,9 @@ #include "mb/pg_wchar.h" #include "nodes/execnodes.h" #include "nodes/makefuncs.h" +#include "pg_extension_base/pg_compat.h" #include "port/pg_bswap.h" +#include "storage/fd.h" #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -198,7 +200,7 @@ CopySendEndOfRow(CopyToState cstate) switch (cstate->copy_dest) { case COPY_FILE: - if (!cstate->opts.binary) + if (!CopyOptsIsBinary(cstate->opts)) { /* Default line termination depends on platform */ #ifndef WIN32 @@ -264,7 +266,7 @@ CopySendInt16(CopyToState cstate, int16 val) static void EndCopy(CopyToState cstate) { - if (cstate->opts.binary) + if (CopyOptsIsBinary(cstate->opts)) { /* Generate trailer for a binary copy */ CopySendInt16(cstate, -1); @@ -461,7 +463,7 @@ StartCopyTo(CopyToState cstate, TupleDesc tupDesc) bool isvarlena; Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1); - if (cstate->opts.binary) + if (CopyOptsIsBinary(cstate->opts)) getTypeBinaryOutputInfo(attr->atttypid, &out_func_oid, &isvarlena); @@ -482,7 +484,7 @@ StartCopyTo(CopyToState cstate, TupleDesc tupDesc) "COPY TO", ALLOCSET_DEFAULT_SIZES); - if (cstate->opts.binary) + if (CopyOptsIsBinary(cstate->opts)) { /* Generate header for a binary copy */ int32 tmp; @@ -523,7 +525,7 @@ StartCopyTo(CopyToState cstate, TupleDesc tupDesc) colname = NameStr(TupleDescAttr(tupDesc, attnum - 1)->attname); - if (cstate->opts.csv_mode) + if (CopyOptsIsCsvMode(cstate->opts)) CopyAttributeOutCSV(cstate, colname, false, list_length(cstate->attnumlist) == 1); else @@ -731,7 +733,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) MemoryContextReset(cstate->rowcontext); oldcontext = MemoryContextSwitchTo(cstate->rowcontext); - if (cstate->opts.binary) + if (CopyOptsIsBinary(cstate->opts)) { /* Binary per-tuple header */ CopySendInt16(cstate, list_length(cstate->attnumlist)); @@ -746,7 +748,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) Datum value = slot->tts_values[attnum - 1]; bool isnull = slot->tts_isnull[attnum - 1]; - if (!cstate->opts.binary) + if (!CopyOptsIsBinary(cstate->opts)) { if (need_delim) CopySendChar(cstate, cstate->opts.delim[0]); @@ -755,14 +757,14 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) if (isnull) { - if (!cstate->opts.binary) + if (!CopyOptsIsBinary(cstate->opts)) CopySendString(cstate, cstate->opts.null_print_client); else CopySendInt32(cstate, -1); } else { - if (!cstate->opts.binary) + if (!CopyOptsIsBinary(cstate->opts)) { /* * Lookup the underlying tuple's attribute so we can pass in @@ -802,7 +804,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) } - if (cstate->opts.csv_mode) + if (CopyOptsIsCsvMode(cstate->opts)) CopyAttributeOutCSV(cstate, string, cstate->opts.force_quote_flags[attnum - 1], list_length(cstate->attnumlist) == 1); diff --git a/pg_lake_engine/src/data_file/data_file_stats.c b/pg_lake_engine/src/data_file/data_file_stats.c index a2df0c7e9..53748b089 100644 --- a/pg_lake_engine/src/data_file/data_file_stats.c +++ b/pg_lake_engine/src/data_file/data_file_stats.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "executor/executor.h" #include "pg_lake/data_file/data_files.h" diff --git a/pg_lake_engine/src/json/json_reader.c b/pg_lake_engine/src/json/json_reader.c index ff54c125f..37623c93f 100644 --- a/pg_lake_engine/src/json/json_reader.c +++ b/pg_lake_engine/src/json/json_reader.c @@ -18,6 +18,7 @@ #include "postgres.h" #include "common/fe_memutils.h" +#include "pg_extension_base/pg_compat.h" #include "pg_lake/json/json_reader.h" #include "utils/builtins.h" #include "utils/jsonb.h" diff --git a/pg_lake_engine/src/parquet/field.c b/pg_lake_engine/src/parquet/field.c index 8acdfb45f..b8f8e3299 100644 --- a/pg_lake_engine/src/parquet/field.c +++ b/pg_lake_engine/src/parquet/field.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "common/int.h" diff --git a/pg_lake_engine/src/parsetree/columns.c b/pg_lake_engine/src/parsetree/columns.c index 87cec1a80..10882ea56 100644 --- a/pg_lake_engine/src/parsetree/columns.c +++ b/pg_lake_engine/src/parsetree/columns.c @@ -16,6 +16,9 @@ */ #include "postgres.h" + +#include "pg_extension_base/pg_compat.h" + #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "nodes/primnodes.h" @@ -80,6 +83,8 @@ BuildTupleDescriptorForTargetList(List *targetList) attributeNumber++; } + TupleDescFinalize(tupleDesc); + return tupleDesc; } diff --git a/pg_lake_engine/src/parsetree/const.c b/pg_lake_engine/src/parsetree/const.c index 46276f348..861c34331 100644 --- a/pg_lake_engine/src/parsetree/const.c +++ b/pg_lake_engine/src/parsetree/const.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "pg_lake/parsetree/const.h" diff --git a/pg_lake_engine/src/parsetree/expression.c b/pg_lake_engine/src/parsetree/expression.c index c22279cb8..9ca195440 100644 --- a/pg_lake_engine/src/parsetree/expression.c +++ b/pg_lake_engine/src/parsetree/expression.c @@ -24,6 +24,7 @@ #include "pg_lake/parsetree/const.h" #include "pg_lake/parsetree/expression.h" #include "datatype/timestamp.h" +#include "utils/timestamp.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "nodes/parsenodes.h" diff --git a/pg_lake_engine/src/pgduck/client.c b/pg_lake_engine/src/pgduck/client.c index e64e6c327..12023e76b 100644 --- a/pg_lake_engine/src/pgduck/client.c +++ b/pg_lake_engine/src/pgduck/client.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "utils/hsearch.h" #include "miscadmin.h" #include "libpq-fe.h" diff --git a/pg_lake_engine/src/pgduck/map.c b/pg_lake_engine/src/pgduck/map.c index 7a28fd0ed..b92c2a973 100644 --- a/pg_lake_engine/src/pgduck/map.c +++ b/pg_lake_engine/src/pgduck/map.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "access/htup_details.h" +#include "catalog/pg_type_d.h" #include "miscadmin.h" #include "libpq-fe.h" diff --git a/pg_lake_engine/src/pgduck/parse_struct.c b/pg_lake_engine/src/pgduck/parse_struct.c index 4c6a47306..3f47e9eeb 100644 --- a/pg_lake_engine/src/pgduck/parse_struct.c +++ b/pg_lake_engine/src/pgduck/parse_struct.c @@ -23,6 +23,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "miscadmin.h" #include "libpq-fe.h" diff --git a/pg_lake_engine/src/pgduck/read_data.c b/pg_lake_engine/src/pgduck/read_data.c index c753e712c..540be6000 100644 --- a/pg_lake_engine/src/pgduck/read_data.c +++ b/pg_lake_engine/src/pgduck/read_data.c @@ -19,6 +19,7 @@ * Functions for generating query for reading data from pgduck server. */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include diff --git a/pg_lake_engine/src/pgduck/rewrite_query.c b/pg_lake_engine/src/pgduck/rewrite_query.c index e37fff63d..5be822e80 100644 --- a/pg_lake_engine/src/pgduck/rewrite_query.c +++ b/pg_lake_engine/src/pgduck/rewrite_query.c @@ -16,6 +16,9 @@ */ #include "postgres.h" +#include "access/htup_details.h" +#include "catalog/pg_type_d.h" +#include "utils/hsearch.h" #include #include diff --git a/pg_lake_engine/src/pgduck/shippable_spatial_functions.c b/pg_lake_engine/src/pgduck/shippable_spatial_functions.c index 49a1e1c59..26d2c3da0 100644 --- a/pg_lake_engine/src/pgduck/shippable_spatial_functions.c +++ b/pg_lake_engine/src/pgduck/shippable_spatial_functions.c @@ -20,6 +20,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "nodes/nodeFuncs.h" #include "optimizer/optimizer.h" diff --git a/pg_lake_engine/src/pgduck/type.c b/pg_lake_engine/src/pgduck/type.c index eb40c5e38..6330b76b5 100644 --- a/pg_lake_engine/src/pgduck/type.c +++ b/pg_lake_engine/src/pgduck/type.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "miscadmin.h" #include "libpq-fe.h" diff --git a/pg_lake_engine/src/pgduck/write_data.c b/pg_lake_engine/src/pgduck/write_data.c index 7ea64e4c6..d904a6712 100644 --- a/pg_lake_engine/src/pgduck/write_data.c +++ b/pg_lake_engine/src/pgduck/write_data.c @@ -19,6 +19,7 @@ * Functions for generating query for writing data via pgduck server. */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "access/tupdesc.h" #include "commands/defrem.h" @@ -29,6 +30,7 @@ #include "pg_lake/extensions/postgis.h" #include "pg_lake/parquet/field.h" #include "pg_lake/parquet/geoparquet.h" +#include "pg_lake/parsetree/options.h" #include "pg_lake/pgduck/keywords.h" #include "pg_lake/pgduck/numeric.h" #include "pg_lake/pgduck/read_data.h" @@ -291,6 +293,11 @@ WriteQueryResultTo(char *query, appendStringInfo(&command, ", compression %s", quote_literal_cstr(compressionName)); + + /* FORCE_ARRAY maps to DuckDB's ARRAY (single JSON array). */ + if (GetBoolOption(formatOptions, "force_array", false)) + appendStringInfoString(&command, ", array true"); + break; } diff --git a/pg_lake_engine/src/query/execute.c b/pg_lake_engine/src/query/execute.c index f4a6aa78e..3bd3730ee 100644 --- a/pg_lake_engine/src/query/execute.c +++ b/pg_lake_engine/src/query/execute.c @@ -75,7 +75,11 @@ uint64 ExecuteQueryToDestReceiver(Query *query, const char *queryString, ParamListInfo params, DestReceiver *dest) { - PlannedStmt *plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, params); + PlannedStmt *plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, params +#if PG_VERSION_NUM >= 190000 + ,NULL +#endif + ); return ExecutePlanToDestReceiver(plan, queryString, params, dest); } diff --git a/pg_lake_engine/src/utils/compare_utils.c b/pg_lake_engine/src/utils/compare_utils.c index 6c2b161f3..278e974b5 100644 --- a/pg_lake_engine/src/utils/compare_utils.c +++ b/pg_lake_engine/src/utils/compare_utils.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "catalog/pg_collation.h" #include "fmgr.h" #include "utils/builtins.h" diff --git a/pg_lake_engine/src/utils/operator_utils.c b/pg_lake_engine/src/utils/operator_utils.c index baa41aa93..a919dc961 100644 --- a/pg_lake_engine/src/utils/operator_utils.c +++ b/pg_lake_engine/src/utils/operator_utils.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "catalog/pg_operator.h" #include "pg_lake/util/operator_utils.h" diff --git a/pg_lake_engine/src/utils/plan_cache.c b/pg_lake_engine/src/utils/plan_cache.c index 166988dd0..efb9bfa7b 100644 --- a/pg_lake_engine/src/utils/plan_cache.c +++ b/pg_lake_engine/src/utils/plan_cache.c @@ -26,6 +26,7 @@ * consider the cache size and eviction policy. */ #include "postgres.h" +#include "utils/hsearch.h" #include "fmgr.h" #include "miscadmin.h" diff --git a/pg_lake_iceberg/Makefile b/pg_lake_iceberg/Makefile index 9ecd68836..8ec2ebc11 100644 --- a/pg_lake_iceberg/Makefile +++ b/pg_lake_iceberg/Makefile @@ -7,7 +7,7 @@ DATA = $(wildcard $(EXTENSION)--*--*.sql) $(EXTENSION)--3.0.sql PG_CONFIG ?= pg_config PG_LIBDIR := $(shell $(PG_CONFIG) --libdir) -PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -std=gnu99 -g -I../pg_lake_engine/include -I../pg_extension_base/include +PG_CPPFLAGS = -I$(libpq_srcdir) -Iinclude -std=gnu11 -g -I../pg_lake_engine/include -I../pg_extension_base/include SHLIB_LINK_INTERNAL = $(libpq) PG_LDFLAGS = -L$(PG_LIBDIR) -Wl,-rpath,$(PG_LIBDIR) -lavro -lcurl diff --git a/pg_lake_iceberg/src/iceberg/api/table_metadata.c b/pg_lake_iceberg/src/iceberg/api/table_metadata.c index 611b3781e..40766f9ac 100644 --- a/pg_lake_iceberg/src/iceberg/api/table_metadata.c +++ b/pg_lake_iceberg/src/iceberg/api/table_metadata.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "storage/fd.h" #include "fmgr.h" #include "funcapi.h" #include "libpq-fe.h" diff --git a/pg_lake_iceberg/src/iceberg/catalog.c b/pg_lake_iceberg/src/iceberg/catalog.c index 7e4cd64f9..e07994dc7 100644 --- a/pg_lake_iceberg/src/iceberg/catalog.c +++ b/pg_lake_iceberg/src/iceberg/catalog.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "miscadmin.h" #include "pg_lake/extensions/pg_lake_iceberg.h" diff --git a/pg_lake_iceberg/src/iceberg/iceberg_type_binary_serde.c b/pg_lake_iceberg/src/iceberg/iceberg_type_binary_serde.c index 67828a27e..463b4ec20 100644 --- a/pg_lake_iceberg/src/iceberg/iceberg_type_binary_serde.c +++ b/pg_lake_iceberg/src/iceberg/iceberg_type_binary_serde.c @@ -16,6 +16,10 @@ */ #include "postgres.h" + +#include + +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "libpq-fe.h" diff --git a/pg_lake_iceberg/src/iceberg/iceberg_type_json_serde.c b/pg_lake_iceberg/src/iceberg/iceberg_type_json_serde.c index 5f0bbf0cd..b37a5aa67 100644 --- a/pg_lake_iceberg/src/iceberg/iceberg_type_json_serde.c +++ b/pg_lake_iceberg/src/iceberg/iceberg_type_json_serde.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "libpq-fe.h" diff --git a/pg_lake_iceberg/src/iceberg/metadata_operations.c b/pg_lake_iceberg/src/iceberg/metadata_operations.c index 0938f5fe2..6aff58a6a 100644 --- a/pg_lake_iceberg/src/iceberg/metadata_operations.c +++ b/pg_lake_iceberg/src/iceberg/metadata_operations.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "utils/hsearch.h" #include "fmgr.h" #include "access/xact.h" #include "common/hashfn.h" diff --git a/pg_lake_iceberg/src/iceberg/operations/find_referenced_files.c b/pg_lake_iceberg/src/iceberg/operations/find_referenced_files.c index f42e8fdb7..3aadec87a 100644 --- a/pg_lake_iceberg/src/iceberg/operations/find_referenced_files.c +++ b/pg_lake_iceberg/src/iceberg/operations/find_referenced_files.c @@ -23,6 +23,7 @@ * referenced in the latest metadata.json file. */ #include "postgres.h" +#include "utils/hsearch.h" #include "miscadmin.h" #include "fmgr.h" #include "funcapi.h" diff --git a/pg_lake_iceberg/src/iceberg/read_manifest.c b/pg_lake_iceberg/src/iceberg/read_manifest.c index 3086eb3f9..6dc8d2a0c 100644 --- a/pg_lake_iceberg/src/iceberg/read_manifest.c +++ b/pg_lake_iceberg/src/iceberg/read_manifest.c @@ -16,11 +16,14 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "libpq-fe.h" #include "miscadmin.h" +#include "storage/fd.h" #include "utils/builtins.h" #include "utils/snapmgr.h" #include "utils/wait_event.h" diff --git a/pg_lake_iceberg/src/object_store_catalog/object_store_catalog.c b/pg_lake_iceberg/src/object_store_catalog/object_store_catalog.c index 8eb6a985e..4c41e33fb 100644 --- a/pg_lake_iceberg/src/object_store_catalog/object_store_catalog.c +++ b/pg_lake_iceberg/src/object_store_catalog/object_store_catalog.c @@ -1,4 +1,5 @@ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "funcapi.h" #include "miscadmin.h" #include "pgtime.h" @@ -9,6 +10,7 @@ #include "utils/snapmgr.h" #include "utils/lsyscache.h" #include "utils/timestamp.h" +#include "utils/tuplestore.h" #include "pg_lake/json/json_utils.h" #include "pg_lake/iceberg/catalog.h" diff --git a/pg_lake_iceberg/src/test/test_http_client.c b/pg_lake_iceberg/src/test/test_http_client.c index 185a77acd..539b2a2b4 100644 --- a/pg_lake_iceberg/src/test/test_http_client.c +++ b/pg_lake_iceberg/src/test/test_http_client.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "fmgr.h" #include "funcapi.h" #include "utils/array.h" diff --git a/pg_lake_iceberg/src/test/test_iceberg_binary_serde.c b/pg_lake_iceberg/src/test/test_iceberg_binary_serde.c index 685aee4a2..92d0a3fbb 100644 --- a/pg_lake_iceberg/src/test/test_iceberg_binary_serde.c +++ b/pg_lake_iceberg/src/test/test_iceberg_binary_serde.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" @@ -28,6 +29,7 @@ #include "utils/builtins.h" #include "utils/json.h" +#include "utils/tuplestore.h" static ColumnBound * FindColumnBoundByColumnId(ColumnBound * bounds, int numBounds, int columnId); diff --git a/pg_lake_iceberg/src/test/test_iceberg_field_ids.c b/pg_lake_iceberg/src/test/test_iceberg_field_ids.c index 1431697a3..f5689cc7a 100644 --- a/pg_lake_iceberg/src/test/test_iceberg_field_ids.c +++ b/pg_lake_iceberg/src/test/test_iceberg_field_ids.c @@ -24,6 +24,7 @@ #include "pg_lake/parquet/leaf_field.h" #include "utils/builtins.h" +#include "utils/tuplestore.h" PG_FUNCTION_INFO_V1(pg_lake_get_leaf_field_ids); diff --git a/pg_lake_iceberg/src/test/test_iceberg_manifest.c b/pg_lake_iceberg/src/test/test_iceberg_manifest.c index 62f4d6921..c0ee48f8a 100644 --- a/pg_lake_iceberg/src/test/test_iceberg_manifest.c +++ b/pg_lake_iceberg/src/test/test_iceberg_manifest.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "libpq-fe.h" @@ -28,6 +29,7 @@ #include "pg_lake/iceberg/partitioning/partition.h" #include "utils/builtins.h" +#include "utils/tuplestore.h" static const char *FetchCurrentSnapshotManifestListPathFromTableMetadataUri(const char *tableMetadataPath); static List *FetchDataFilePathsFromTableMetadataUri(const char *tableMetadataPath, bool isDelete); diff --git a/pg_lake_iceberg/src/utils/truncate_utils.c b/pg_lake_iceberg/src/utils/truncate_utils.c index e136e4cab..73a333b5e 100644 --- a/pg_lake_iceberg/src/utils/truncate_utils.c +++ b/pg_lake_iceberg/src/utils/truncate_utils.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "pg_extension_base/pg_compat.h" #include "mb/pg_wchar.h" #include "utils/builtins.h" #include "utils/fmgrprotos.h" diff --git a/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h b/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h index 65e5a9c25..5db9e2d4a 100644 --- a/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h +++ b/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h @@ -22,7 +22,9 @@ #include "pg_lake/util/s3_reader_utils.h" #include "nodes/pg_list.h" +#if PG_VERSION_NUM < 190000 #include "utils/dynahash.h" +#endif #define DATA_FILES_TABLE_QUALIFIED \ PG_LAKE_TABLE_SCHEMA "." PG_LAKE_TABLE_FILES_TABLE_NAME diff --git a/pg_lake_table/include/pg_lake/fdw/shippable.h b/pg_lake_table/include/pg_lake/fdw/shippable.h index 9335aa21d..051c54903 100644 --- a/pg_lake_table/include/pg_lake/fdw/shippable.h +++ b/pg_lake_table/include/pg_lake/fdw/shippable.h @@ -41,6 +41,7 @@ typedef enum NotShippableReason NOT_SHIPPABLE_SQL_WITH_ORDINALITY, NOT_SHIPPABLE_SQL_JOIN_MERGED_COLUMNS_ALIAS, NOT_SHIPPABLE_SQL_UNNEST_GROUP_BY_OR_WINDOW, + NOT_SHIPPABLE_SQL_GROUP_BY_ALL, NOT_SHIPPABLE_INFINITE_CONST_VALUE } NotShippableReason; diff --git a/pg_lake_table/src/ddl/alter_table.c b/pg_lake_table/src/ddl/alter_table.c index ad03e44ce..7a801f504 100644 --- a/pg_lake_table/src/ddl/alter_table.c +++ b/pg_lake_table/src/ddl/alter_table.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "miscadmin.h" #include "access/heapam.h" diff --git a/pg_lake_table/src/ddl/create_table.c b/pg_lake_table/src/ddl/create_table.c index 3ee954264..4e5616ff8 100644 --- a/pg_lake_table/src/ddl/create_table.c +++ b/pg_lake_table/src/ddl/create_table.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "miscadmin.h" #include "access/table.h" diff --git a/pg_lake_table/src/describe/describe.c b/pg_lake_table/src/describe/describe.c index 5a32d431f..8d4c78773 100644 --- a/pg_lake_table/src/describe/describe.c +++ b/pg_lake_table/src/describe/describe.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "libpq-fe.h" #include "commands/defrem.h" diff --git a/pg_lake_table/src/duckdb/transform_query_to_duckdb.c b/pg_lake_table/src/duckdb/transform_query_to_duckdb.c index f0fb4dfbd..f30b5d5d6 100644 --- a/pg_lake_table/src/duckdb/transform_query_to_duckdb.c +++ b/pg_lake_table/src/duckdb/transform_query_to_duckdb.c @@ -28,6 +28,7 @@ #include "access/relation.h" #include "access/xact.h" #include "utils/builtins.h" +#include "utils/timestamp.h" #include "catalog/pg_class_d.h" #include "catalog/pg_type_d.h" #include "pg_lake/iceberg/api/table_schema.h" diff --git a/pg_lake_table/src/fdw/data_file_pruning.c b/pg_lake_table/src/fdw/data_file_pruning.c index b2beca68a..bd287d295 100644 --- a/pg_lake_table/src/fdw/data_file_pruning.c +++ b/pg_lake_table/src/fdw/data_file_pruning.c @@ -21,6 +21,9 @@ * execution and statistics of the data files. */ #include "postgres.h" +#include "utils/hsearch.h" +#include "access/htup_details.h" +#include "catalog/pg_type_d.h" #include "catalog/pg_am_d.h" #include "catalog/pg_index.h" @@ -1565,7 +1568,15 @@ ColumnsUsedInRestrictions(Oid relationId, List *baseRestrictInfoList) RestrictInfo *restrictInfo = lfirst(restrictCell); Node *qual = (Node *) restrictInfo->clause; - List *varForQual = pull_var_clause(qual, PVC_INCLUDE_PLACEHOLDERS); + /* + * Recurse into Aggref/PlaceHolderVar so we still see the underlying + * Vars used in any pushed-up aggregate or PHV; PG19 can leave such + * nodes inside baserestrictinfo clauses where earlier majors did not, + * and pull_var_clause without a recurse flag elog()s on them. + */ + List *varForQual = pull_var_clause(qual, + PVC_RECURSE_AGGREGATES | + PVC_RECURSE_PLACEHOLDERS); ListCell *varCell = NULL; diff --git a/pg_lake_table/src/fdw/data_files_catalog.c b/pg_lake_table/src/fdw/data_files_catalog.c index 5890683b1..82d343813 100644 --- a/pg_lake_table/src/fdw/data_files_catalog.c +++ b/pg_lake_table/src/fdw/data_files_catalog.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "miscadmin.h" diff --git a/pg_lake_table/src/fdw/data_files_catalog_batch.c b/pg_lake_table/src/fdw/data_files_catalog_batch.c index 750684a93..21d275529 100644 --- a/pg_lake_table/src/fdw/data_files_catalog_batch.c +++ b/pg_lake_table/src/fdw/data_files_catalog_batch.c @@ -25,6 +25,8 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "pg_extension_base/extension_ids.h" #include "pg_extension_base/spi_helpers.h" @@ -42,7 +44,9 @@ #include "executor/spi.h" #include "utils/builtins.h" +#if PG_VERSION_NUM < 190000 #include "utils/dynahash.h" +#endif /* path -> int64 file id, built from RETURNING output of ExecInsertDataFiles */ typedef struct FileIdHashEntry diff --git a/pg_lake_table/src/fdw/deparse.c b/pg_lake_table/src/fdw/deparse.c index 48c36da49..4f9325f6f 100644 --- a/pg_lake_table/src/fdw/deparse.c +++ b/pg_lake_table/src/fdw/deparse.c @@ -1222,7 +1222,11 @@ is_foreign_pathkey(PlannerInfo *root, static char * deparse_type_name(Oid type_oid, int32 typemod) { +#if PG_VERSION_NUM >= 190000 + uint16 flags = FORMAT_TYPE_TYPEMOD_GIVEN; +#else bits16 flags = FORMAT_TYPE_TYPEMOD_GIVEN; +#endif if (!is_builtin(type_oid)) flags |= FORMAT_TYPE_FORCE_QUALIFY; @@ -1258,14 +1262,16 @@ build_tlist_to_deparse(RelOptInfo *foreignrel) */ tlist = add_to_flat_tlist(tlist, pull_var_clause((Node *) foreignrel->reltarget->exprs, - PVC_RECURSE_PLACEHOLDERS)); + PVC_RECURSE_PLACEHOLDERS | + PVC_RECURSE_AGGREGATES)); foreach(lc, fpinfo->local_conds) { RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc); tlist = add_to_flat_tlist(tlist, pull_var_clause((Node *) rinfo->clause, - PVC_RECURSE_PLACEHOLDERS)); + PVC_RECURSE_PLACEHOLDERS | + PVC_RECURSE_AGGREGATES)); } return tlist; diff --git a/pg_lake_table/src/fdw/deparse_ruleutils.c b/pg_lake_table/src/fdw/deparse_ruleutils.c index 9e3c565fe..6b7b4c3c0 100644 --- a/pg_lake_table/src/fdw/deparse_ruleutils.c +++ b/pg_lake_table/src/fdw/deparse_ruleutils.c @@ -25,6 +25,7 @@ *------------------------------------------------------------------------- */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "parser/parse_func.h" #include "parser/parsetree.h" diff --git a/pg_lake_table/src/fdw/partition_pushdown.c b/pg_lake_table/src/fdw/partition_pushdown.c index 2b0071a98..5a0f804a8 100644 --- a/pg_lake_table/src/fdw/partition_pushdown.c +++ b/pg_lake_table/src/fdw/partition_pushdown.c @@ -24,6 +24,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "executor/executor.h" #include "utils/array.h" #include "utils/builtins.h" diff --git a/pg_lake_table/src/fdw/partition_transform.c b/pg_lake_table/src/fdw/partition_transform.c index 6d7035698..29129d318 100644 --- a/pg_lake_table/src/fdw/partition_transform.c +++ b/pg_lake_table/src/fdw/partition_transform.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "utils/builtins.h" #include "utils/date.h" #include "utils/lsyscache.h" diff --git a/pg_lake_table/src/fdw/partitioning/partition_spec_catalog.c b/pg_lake_table/src/fdw/partitioning/partition_spec_catalog.c index ccee948a0..42a2e41ec 100644 --- a/pg_lake_table/src/fdw/partitioning/partition_spec_catalog.c +++ b/pg_lake_table/src/fdw/partitioning/partition_spec_catalog.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "pg_lake/extensions/pg_lake_table.h" diff --git a/pg_lake_table/src/fdw/partitioning/partitioned_dest_receiver.c b/pg_lake_table/src/fdw/partitioning/partitioned_dest_receiver.c index 8f34c30cc..45cb5a4ab 100644 --- a/pg_lake_table/src/fdw/partitioning/partitioned_dest_receiver.c +++ b/pg_lake_table/src/fdw/partitioning/partitioned_dest_receiver.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "utils/hsearch.h" #include "access/tupdesc.h" #include "commands/copy.h" diff --git a/pg_lake_table/src/fdw/pg_lake_table.c b/pg_lake_table/src/fdw/pg_lake_table.c index 507949e2d..485b7428c 100644 --- a/pg_lake_table/src/fdw/pg_lake_table.c +++ b/pg_lake_table/src/fdw/pg_lake_table.c @@ -25,6 +25,8 @@ *------------------------------------------------------------------------- */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "funcapi.h" #include @@ -2102,6 +2104,15 @@ AddDeleteReturningTargetTableVars(PlannerInfo *root, if (returningVar->varno != root->parse->resultRelation) continue; + /* + * Skip whole-row references (handled above) and system columns; the + * latter have negative varattno and would index out of bounds in + * TupleDescAttr() below — PG19 added an Assert that fires on + * negative indexes where earlier majors silently returned garbage. + */ + if (returningVar->varattno <= 0) + continue; + #if PG_VERSION_NUM >= 180000 if (returningVar->varreturningtype == VAR_RETURNING_NEW || returningVar->varreturningtype == VAR_RETURNING_OLD) @@ -2771,6 +2782,8 @@ CreateRowIdTupleDesc(void) TupleDescInitEntry(tupleDescriptor, (AttrNumber) 2, "file_row_number", INT8OID, -1, 0); + TupleDescFinalize(tupleDescriptor); + return tupleDescriptor; } @@ -3915,7 +3928,16 @@ semijoin_target_ok(PlannerInfo *root, RelOptInfo *joinrel, RelOptInfo *outerrel, Assert(joinrel->reltarget); - vars = pull_var_clause((Node *) joinrel->reltarget->exprs, PVC_INCLUDE_PLACEHOLDERS); + /* + * Diverges from upstream postgres_fdw (which uses + * PVC_INCLUDE_PLACEHOLDERS only, unchanged in PG18/PG19): pg_lake can + * leave an Aggref in a SEMI join's target exprs (e.g. TPC-H Q21). Recurse + * into it so we still see the underlying Vars and reject inner-rel + * references instead of elog-ing. + */ + vars = pull_var_clause((Node *) joinrel->reltarget->exprs, + PVC_INCLUDE_PLACEHOLDERS | + PVC_RECURSE_AGGREGATES); foreach(lc, vars) { @@ -4304,10 +4326,17 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel, PgLakeRelationInfo *fpinfo = (PgLakeRelationInfo *) rel->fdw_private; PathTarget *target = copy_pathtarget(epq_path->pathtarget); - /* Include columns required for evaluating PHVs in the tlist. */ + /* + * Include columns required for evaluating PHVs, plus the underlying + * Vars of any Aggref in the tlist. Diverges from upstream + * postgres_fdw (PVC_RECURSE_PLACEHOLDERS only): pg_lake's EPQ path + * targets can hold an Aggref (e.g. TPC-H Q21 / TPC-DS Q10), where + * pull_var_clause would otherwise elog(). + */ add_new_columns_to_pathtarget(target, pull_var_clause((Node *) target->exprs, - PVC_RECURSE_PLACEHOLDERS)); + PVC_RECURSE_PLACEHOLDERS | + PVC_RECURSE_AGGREGATES)); /* Include columns required for evaluating the local conditions. */ foreach(lc, fpinfo->local_conds) @@ -4316,7 +4345,8 @@ add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel, add_new_columns_to_pathtarget(target, pull_var_clause((Node *) rinfo->clause, - PVC_RECURSE_PLACEHOLDERS)); + PVC_RECURSE_PLACEHOLDERS | + PVC_RECURSE_AGGREGATES)); } /* diff --git a/pg_lake_table/src/fdw/position_delete_dest.c b/pg_lake_table/src/fdw/position_delete_dest.c index 21b3fb564..66e204691 100644 --- a/pg_lake_table/src/fdw/position_delete_dest.c +++ b/pg_lake_table/src/fdw/position_delete_dest.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "access/htup_details.h" +#include "utils/hsearch.h" #include "access/tupdesc.h" #include "commands/copy.h" diff --git a/pg_lake_table/src/fdw/schema_operations/field_id_mapping_catalog.c b/pg_lake_table/src/fdw/schema_operations/field_id_mapping_catalog.c index 53dabb678..ce3d0ffe7 100644 --- a/pg_lake_table/src/fdw/schema_operations/field_id_mapping_catalog.c +++ b/pg_lake_table/src/fdw/schema_operations/field_id_mapping_catalog.c @@ -22,6 +22,7 @@ * from/to catalog lake_table.field_id_mappings. */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "miscadmin.h" #include "access/relation.h" diff --git a/pg_lake_table/src/fdw/schema_operations/register_field_ids.c b/pg_lake_table/src/fdw/schema_operations/register_field_ids.c index ec6d00a51..2fe80c2f7 100644 --- a/pg_lake_table/src/fdw/schema_operations/register_field_ids.c +++ b/pg_lake_table/src/fdw/schema_operations/register_field_ids.c @@ -275,11 +275,24 @@ CreatePostgresColumnMappingsForIcebergTableFromExternalMetadata(Oid relationId) columnMapping->attname = pstrdup(field->name); columnMapping->attrNum = get_attnum(relationId, field->name); - Form_pg_attribute attr = TupleDescAttr(tupDesc, columnMapping->attrNum - 1); - columnMapping->pgType = MakePGType(attr->atttypid, attr->atttypmod); - columnMapping->attNotNull = attr->attnotnull; - columnMapping->attHasDef = attr->atthasdef; + if (columnMapping->attrNum != InvalidAttrNumber) + { + Form_pg_attribute attr = TupleDescAttr(tupDesc, columnMapping->attrNum - 1); + + columnMapping->pgType = MakePGType(attr->atttypid, attr->atttypmod); + columnMapping->attNotNull = attr->attnotnull; + columnMapping->attHasDef = attr->atthasdef; + } + else + { + /* + * No matching Postgres column. Leave the type/null/default fields + * zero so ErrorIfSchemasDoNotMatch() reports the mismatch, and + * avoid TupleDescAttr(InvalidAttrNumber - 1), which trips PG19's + * new bounds Assert in TupleDescAttr(). + */ + } pgColumnMappingList = lappend(pgColumnMappingList, columnMapping); } diff --git a/pg_lake_table/src/fdw/shippable.c b/pg_lake_table/src/fdw/shippable.c index aa065c7a4..ed5ff18f3 100644 --- a/pg_lake_table/src/fdw/shippable.c +++ b/pg_lake_table/src/fdw/shippable.c @@ -708,6 +708,8 @@ GetNotShippableDescription(NotShippableReason reason, Oid classId, Oid objectId) return "JOIN with merged columns and alias"; case NOT_SHIPPABLE_SQL_UNNEST_GROUP_BY_OR_WINDOW: return "UNNEST with GROUP BY or window function"; + case NOT_SHIPPABLE_SQL_GROUP_BY_ALL: + return "GROUP BY ALL"; case NOT_SHIPPABLE_INFINITE_CONST_VALUE: return "infinite interval constant"; default: diff --git a/pg_lake_table/src/fdw/update_tracking.c b/pg_lake_table/src/fdw/update_tracking.c index b206cd1f3..b46fe1b41 100644 --- a/pg_lake_table/src/fdw/update_tracking.c +++ b/pg_lake_table/src/fdw/update_tracking.c @@ -219,7 +219,11 @@ CreateUpdateTrackingTable(RangeVar *updateTableName) indexColumn->name = "rowid"; createPrimaryKey->indexParams = list_make1(indexColumn); - DefineIndex(updateTableAddress.objectId, + DefineIndex( +#if PG_VERSION_NUM >= 190000 + /* pstate */ NULL, +#endif + updateTableAddress.objectId, createPrimaryKey, /* indexRelationId */ InvalidOid, /* parentIndexId */ InvalidOid, diff --git a/pg_lake_table/src/fdw/writable_table.c b/pg_lake_table/src/fdw/writable_table.c index 5b1f3f970..f79438325 100644 --- a/pg_lake_table/src/fdw/writable_table.c +++ b/pg_lake_table/src/fdw/writable_table.c @@ -16,6 +16,9 @@ */ #include "postgres.h" +#include "storage/lock.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "fmgr.h" #include "funcapi.h" #include "miscadmin.h" @@ -26,6 +29,7 @@ #include "catalog/pg_namespace.h" #include "commands/defrem.h" #include "common/hashfn.h" +#include "pg_extension_base/pg_compat.h" #include "pg_lake/cleanup/in_progress_files.h" #include "pg_lake/data_file/data_files.h" #include "pg_lake/data_file/data_file_stats.h" @@ -1647,6 +1651,8 @@ CreatePositionDeleteTupleDesc(void) TupleDescInitEntry(tupleDescriptor, (AttrNumber) 3, "row", RECORDOID, -1, 0); + TupleDescFinalize(tupleDescriptor); + return tupleDescriptor; } diff --git a/pg_lake_table/src/planner/explain.c b/pg_lake_table/src/planner/explain.c index 827ea0b63..f96c85c94 100644 --- a/pg_lake_table/src/planner/explain.c +++ b/pg_lake_table/src/planner/explain.c @@ -139,6 +139,7 @@ NotShippableObjectToString(const NotShippableObject * notShippableObject) case NOT_SHIPPABLE_SQL_WITH_ORDINALITY: case NOT_SHIPPABLE_SQL_JOIN_MERGED_COLUMNS_ALIAS: case NOT_SHIPPABLE_SQL_UNNEST_GROUP_BY_OR_WINDOW: + case NOT_SHIPPABLE_SQL_GROUP_BY_ALL: appendStringInfo(message, "\tSQL Syntax\n"); break; case NOT_SHIPPABLE_SYSTEM_COLUMN: diff --git a/pg_lake_table/src/planner/insert_select.c b/pg_lake_table/src/planner/insert_select.c index 2c8b7444a..e9682a043 100644 --- a/pg_lake_table/src/planner/insert_select.c +++ b/pg_lake_table/src/planner/insert_select.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "catalog/pg_type_d.h" #include "access/table.h" #include "pg_lake/extensions/postgis.h" diff --git a/pg_lake_table/src/planner/query_pushdown.c b/pg_lake_table/src/planner/query_pushdown.c index 18c9fbe18..23e4fef84 100644 --- a/pg_lake_table/src/planner/query_pushdown.c +++ b/pg_lake_table/src/planner/query_pushdown.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "catalog/pg_type_d.h" #include "funcapi.h" #include "miscadmin.h" @@ -95,7 +97,11 @@ typedef struct IsShippableContext static PlannedStmt *LakeTablePlanner(Query *parse, const char *queryString, - int cursorOptions, ParamListInfo boundParams); + int cursorOptions, ParamListInfo boundParams +#if PG_VERSION_NUM >= 190000 + ,ExplainState *es +#endif +); static bool AdjustParseTreeForPgLake(Node *node, void *context); static bool ProcessNotShippableExpressionWalker(Node *node, IsShippableContext * context); static bool AddMissingRTEAliasaes(Node *node, void *context); @@ -253,7 +259,11 @@ AppendPermInfos(PlannedStmt *pushdownPlan, PlannedStmt *localPlan) */ static PlannedStmt * LakeTablePlanner(Query *parse, const char *queryString, - int cursorOptions, ParamListInfo boundParams) + int cursorOptions, ParamListInfo boundParams +#if PG_VERSION_NUM >= 190000 + ,ExplainState *es +#endif +) { Query *originalQuery = NULL; bool hasLakeTable = false; @@ -293,7 +303,11 @@ LakeTablePlanner(Query *parse, const char *queryString, PG_TRY(); { - plan = PreviousPlannerHook(parse, queryString, cursorOptions, boundParams); + plan = PreviousPlannerHook(parse, queryString, cursorOptions, boundParams +#if PG_VERSION_NUM >= 190000 + ,es +#endif + ); if (EnableFullQueryPushdown && hasLakeTable && (cursorOptions & CURSOR_OPT_SCROLL) == 0) @@ -655,6 +669,26 @@ ProcessNotShippableExpressionWalker(Node *node, IsShippableContext * context) RecordNotShippableObject(context, InvalidOid, InvalidOid, NOT_SHIPPABLE_SQL_EMPTY_TARGET_LIST); } +#if PG_VERSION_NUM >= 190000 + + /* + * Both PostgreSQL (PG19+) and DuckDB accept GROUP BY ALL, but they + * infer the grouping columns from the target list independently and + * the two algorithms diverge (e.g. DuckDB rejects "SELECT b, count(*) + * ... GROUP BY ALL" that PostgreSQL accepts). Deparsing preserves the + * literal GROUP BY ALL, so pushing it down would let DuckDB silently + * re-infer a different grouping. Run such queries locally instead; + * PostgreSQL has already resolved the grouping at parse time. + */ + if (query->groupByAll) + { + if (context->stopAtFirstNotShippable) + return true; + + RecordNotShippableObject(context, InvalidOid, InvalidOid, NOT_SHIPPABLE_SQL_GROUP_BY_ALL); + } +#endif + if (query_tree_walker(query, ProcessNotShippableExpressionWalker, context, QTW_EXAMINE_RTES_BEFORE)) return true; @@ -944,6 +978,23 @@ ExpressionHasNonShippableObject(Node *node, bool srfAllowed, IsShippableContext TryRecordNotShippableObject(context, expr->winfnoid, ProcedureRelationId, NOT_SHIPPABLE_FUNCTION); return true; } + +#if PG_VERSION_NUM >= 190000 + + /* + * PG19 added IGNORE NULLS / RESPECT NULLS null treatment to window + * functions. pg_get_querydef emits the clause after the argument list + * ("lag(v) IGNORE NULLS OVER ..."), which DuckDB's parser rejects, so + * any window function carrying an explicit null treatment must run + * locally. RESPECT NULLS is the default and collapses to + * NO_NULLTREATMENT during parse analysis, so it is unaffected. + */ + if (expr->ignore_nulls != NO_NULLTREATMENT) + { + TryRecordNotShippableObject(context, expr->winfnoid, ProcedureRelationId, NOT_SHIPPABLE_FUNCTION); + return true; + } +#endif } else if (IsA(node, CoerceViaIO)) { diff --git a/pg_lake_table/src/planner/restriction_collector.c b/pg_lake_table/src/planner/restriction_collector.c index 31087de91..6a6f8df52 100644 --- a/pg_lake_table/src/planner/restriction_collector.c +++ b/pg_lake_table/src/planner/restriction_collector.c @@ -24,6 +24,7 @@ * that Postgres planner knows about. */ #include "postgres.h" +#include "utils/hsearch.h" #include "funcapi.h" #include "miscadmin.h" @@ -526,7 +527,16 @@ GenerateSyntheticTestQueryForRestrictInfo(int logLevel, RangeTblEntry *rte, * We'll only have a single RTE in the query, so we can set varno and * varnosyn to 1. */ - List *varList = pull_var_clause((Node *) copyOfRestrictClauses, 0); + + /* + * Recurse into aggregate arguments so we still rewrite the Vars inside + * them. PG19's planner can leave Aggrefs inside FDW-pushdown restriction + * clauses where earlier majors did not, and pull_var_clause(... 0) + * elog()s when it sees an Aggref. + */ + List *varList = pull_var_clause((Node *) copyOfRestrictClauses, + PVC_RECURSE_AGGREGATES | + PVC_RECURSE_PLACEHOLDERS); ListCell *cell = NULL; foreach(cell, varList) diff --git a/pg_lake_table/src/recovery/recover.c b/pg_lake_table/src/recovery/recover.c index 08a485cdb..4d9d1af9c 100644 --- a/pg_lake_table/src/recovery/recover.c +++ b/pg_lake_table/src/recovery/recover.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "miscadmin.h" #include "fmgr.h" diff --git a/pg_lake_table/src/test/hide_lake_objects.c b/pg_lake_table/src/test/hide_lake_objects.c index d53835729..a4e70fe39 100644 --- a/pg_lake_table/src/test/hide_lake_objects.c +++ b/pg_lake_table/src/test/hide_lake_objects.c @@ -16,6 +16,8 @@ */ #include "postgres.h" +#include "utils/hsearch.h" +#include "access/htup_details.h" #include "miscadmin.h" #include "access/genam.h" diff --git a/pg_lake_table/src/test/test_partition_tuple.c b/pg_lake_table/src/test/test_partition_tuple.c index acd2644a8..2bef4fcc5 100644 --- a/pg_lake_table/src/test/test_partition_tuple.c +++ b/pg_lake_table/src/test/test_partition_tuple.c @@ -16,11 +16,14 @@ */ #include "postgres.h" +#include "access/htup_details.h" +#include "catalog/pg_type_d.h" #include "funcapi.h" #include "access/relation.h" #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/rel.h" +#include "utils/tuplestore.h" #include "pg_lake/partitioning/partition_spec_catalog.h" #include "pg_lake/fdw/partition_transform.h" diff --git a/pg_lake_table/src/transaction/track_iceberg_metadata_changes.c b/pg_lake_table/src/transaction/track_iceberg_metadata_changes.c index 69ec66b5e..fddc504a4 100644 --- a/pg_lake_table/src/transaction/track_iceberg_metadata_changes.c +++ b/pg_lake_table/src/transaction/track_iceberg_metadata_changes.c @@ -16,6 +16,7 @@ */ #include "postgres.h" +#include "utils/hsearch.h" #include "access/xact.h" #include "common/int.h" #include "utils/memutils.h" diff --git a/pg_lake_table/tests/isolation/expected/isolation_iceberg_repeatable_read.out b/pg_lake_table/tests/isolation/expected/isolation_iceberg_repeatable_read.out index 2ccc553d9..d3fc61615 100644 --- a/pg_lake_table/tests/isolation/expected/isolation_iceberg_repeatable_read.out +++ b/pg_lake_table/tests/isolation/expected/isolation_iceberg_repeatable_read.out @@ -98,13 +98,17 @@ step s1-update: UPDATE test_iceberg_rr SET value = 99 WHERE key = 1; step s2-update: - UPDATE test_iceberg_rr SET value = 88 WHERE key = 1; + DO $$ BEGIN + UPDATE test_iceberg_rr SET value = 88 WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-update: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -114,13 +118,17 @@ step s1-delete: DELETE FROM test_iceberg_rr WHERE key = 2; step s2-delete: - DELETE FROM test_iceberg_rr WHERE key = 2; + DO $$ BEGIN + DELETE FROM test_iceberg_rr WHERE key = 2; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-delete: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -130,13 +138,17 @@ step s1-insert: INSERT INTO test_iceberg_rr VALUES (4, 40); step s2-insert: - INSERT INTO test_iceberg_rr VALUES (5, 50); + DO $$ BEGIN + INSERT INTO test_iceberg_rr VALUES (5, 50); + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-insert: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -146,13 +158,17 @@ step s1-insert: INSERT INTO test_iceberg_rr VALUES (4, 40); step s2-delete: - DELETE FROM test_iceberg_rr WHERE key = 2; + DO $$ BEGIN + DELETE FROM test_iceberg_rr WHERE key = 2; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-delete: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -162,13 +178,17 @@ step s1-insert: INSERT INTO test_iceberg_rr VALUES (4, 40); step s2-update: - UPDATE test_iceberg_rr SET value = 88 WHERE key = 1; + DO $$ BEGIN + UPDATE test_iceberg_rr SET value = 88 WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-update: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -178,13 +198,17 @@ step s1-update: UPDATE test_iceberg_rr SET value = 99 WHERE key = 1; step s2-delete-key-1: - DELETE FROM test_iceberg_rr WHERE key = 1; + DO $$ BEGIN + DELETE FROM test_iceberg_rr WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-delete-key-1: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; diff --git a/pg_lake_table/tests/isolation/expected/isolation_iceberg_serializable.out b/pg_lake_table/tests/isolation/expected/isolation_iceberg_serializable.out index 2bc90d7fb..63acf63bf 100644 --- a/pg_lake_table/tests/isolation/expected/isolation_iceberg_serializable.out +++ b/pg_lake_table/tests/isolation/expected/isolation_iceberg_serializable.out @@ -22,13 +22,17 @@ step s1-update-key-1: UPDATE test_iceberg_ser SET value = value - 50 WHERE key = 1; step s2-update-key-2: - UPDATE test_iceberg_ser SET value = value - 50 WHERE key = 2; + DO $$ BEGIN + UPDATE test_iceberg_ser SET value = value - 50 WHERE key = 2; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-update-key-2: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -38,13 +42,17 @@ step s1-update-key-1: UPDATE test_iceberg_ser SET value = value - 50 WHERE key = 1; step s2-update-key-1: - UPDATE test_iceberg_ser SET value = value + 10 WHERE key = 1; + DO $$ BEGIN + UPDATE test_iceberg_ser SET value = value + 10 WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-update-key-1: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; @@ -72,13 +80,17 @@ step s1-insert: INSERT INTO test_iceberg_ser VALUES (3, 30); step s2-insert: - INSERT INTO test_iceberg_ser VALUES (4, 40); + DO $$ BEGIN + INSERT INTO test_iceberg_ser VALUES (4, 40); + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; step s1-commit: COMMIT; step s2-insert: <... completed> -ERROR: could not serialize access due to concurrent update +ERROR: could not serialize access step s2-rollback: ROLLBACK; diff --git a/pg_lake_table/tests/isolation/specs/isolation_iceberg_repeatable_read.spec b/pg_lake_table/tests/isolation/specs/isolation_iceberg_repeatable_read.spec index b6f10fa23..5f8d03ae3 100644 --- a/pg_lake_table/tests/isolation/specs/isolation_iceberg_repeatable_read.spec +++ b/pg_lake_table/tests/isolation/specs/isolation_iceberg_repeatable_read.spec @@ -44,24 +44,46 @@ step "s2-select-all" SELECT * FROM test_iceberg_rr ORDER BY key; } +# s2 is always the losing session in these permutations. PG19 distinguishes +# "concurrent update" from "concurrent delete" in the serialization-failure +# message (and pg_lake's copy-on-write makes an UPDATE look like a delete to a +# concurrent reader), so the raw text differs across versions. Trap the failure +# by SQLSTATE (40001) and re-raise one canonical message so a single expected +# file matches every PostgreSQL version. step "s2-insert" { - INSERT INTO test_iceberg_rr VALUES (5, 50); + DO $$ BEGIN + INSERT INTO test_iceberg_rr VALUES (5, 50); + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-update" { - UPDATE test_iceberg_rr SET value = 88 WHERE key = 1; + DO $$ BEGIN + UPDATE test_iceberg_rr SET value = 88 WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-delete" { - DELETE FROM test_iceberg_rr WHERE key = 2; + DO $$ BEGIN + DELETE FROM test_iceberg_rr WHERE key = 2; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-delete-key-1" { - DELETE FROM test_iceberg_rr WHERE key = 1; + DO $$ BEGIN + DELETE FROM test_iceberg_rr WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-commit" diff --git a/pg_lake_table/tests/isolation/specs/isolation_iceberg_serializable.spec b/pg_lake_table/tests/isolation/specs/isolation_iceberg_serializable.spec index 21ef85205..d504e4991 100644 --- a/pg_lake_table/tests/isolation/specs/isolation_iceberg_serializable.spec +++ b/pg_lake_table/tests/isolation/specs/isolation_iceberg_serializable.spec @@ -54,19 +54,37 @@ step "s2-select-sum" SELECT SUM(value) FROM test_iceberg_ser; } +# s2 is always the losing session in these permutations. PG19 distinguishes +# "concurrent update" from "concurrent delete" in the serialization-failure +# message (and pg_lake's copy-on-write makes an UPDATE look like a delete to a +# concurrent reader), so the raw text differs across versions. Trap the failure +# by SQLSTATE (40001) and re-raise one canonical message so a single expected +# file matches every PostgreSQL version. step "s2-update-key-2" { - UPDATE test_iceberg_ser SET value = value - 50 WHERE key = 2; + DO $$ BEGIN + UPDATE test_iceberg_ser SET value = value - 50 WHERE key = 2; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-update-key-1" { - UPDATE test_iceberg_ser SET value = value + 10 WHERE key = 1; + DO $$ BEGIN + UPDATE test_iceberg_ser SET value = value + 10 WHERE key = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-insert" { - INSERT INTO test_iceberg_ser VALUES (4, 40); + DO $$ BEGIN + INSERT INTO test_iceberg_ser VALUES (4, 40); + EXCEPTION WHEN serialization_failure THEN + RAISE EXCEPTION 'could not serialize access' USING ERRCODE = 'serialization_failure'; + END $$; } step "s2-commit" diff --git a/pg_lake_table/tests/pytests/test_grouping_set_pushdown.py b/pg_lake_table/tests/pytests/test_grouping_set_pushdown.py index 3014d6dbc..beb652b3c 100644 --- a/pg_lake_table/tests/pytests/test_grouping_set_pushdown.py +++ b/pg_lake_table/tests/pytests/test_grouping_set_pushdown.py @@ -154,3 +154,57 @@ def test_grouping_sets_pushdown2(pg_conn, with_default_location): assert_remote_query_contains_expression(query, "ROLLUP", pg_conn), query else: assert False + + +# PG19 commit 415100aa6 relaxed parse analysis so that references to a non-Var +# GROUP BY *expression* (and GROUPING()) inside a subquery at the grouping +# query level are accepted instead of being rejected with "subquery uses +# ungrouped column from outer query". These shapes only parse on PG19+, so the +# value here is that pg_lake returns results identical to a heap baseline (the +# query may push down or run locally; either way it must be correct). +_PG19_GROUPING_SUBQUERY_QUERIES = [ + # grouping-EXPRESSION (id + v) referenced inside a correlated sublink + "SELECT id + v AS g, (SELECT max(s.id) FROM gstest5 s WHERE s.id = (t.id + t.v)) " + "FROM gstest5 t GROUP BY id + v ORDER BY 1;", + # the outer grouping expression referenced inside the subquery's WHERE, + # mirroring the commit's tenk1/int4_tbl regression query + "SELECT id + v AS g, (SELECT count(*) FROM gstest5 s WHERE s.v < (t.id + t.v)) AS c " + "FROM gstest5 t GROUP BY id + v ORDER BY 1;", + # GROUPING() and grouping columns referenced inside subqueries, with + # grouping sets (the commit's groupingsets.sql regression case) + "SELECT (SELECT t.id) AS a, (SELECT t.v) AS b, (SELECT GROUPING(t.id, t.v)) AS g, " + "(SELECT SUM(t.id)) AS s FROM gstest5 t GROUP BY GROUPING SETS ((id, v), ()) " + "ORDER BY 1, 2, 3;", +] + + +def test_pg19_grouping_ref_in_subquery(pg_conn, with_default_location): + """PG19 415100aa6: grouping-expression / GROUPING() references inside + subqueries must yield results identical to a heap baseline on iceberg + tables. Skipped pre-PG19 where the parser rejects these queries.""" + if get_pg_version_num(pg_conn) < 190000: + pytest.skip("PG19 relaxed grouping-expression references in subqueries") + + run_command( + """ + DROP TABLE IF EXISTS gstest5; + DROP TABLE IF EXISTS gstest5_pg; + create table gstest5(id integer, v integer) USING iceberg; + insert into gstest5 select i, i % 3 from generate_series(1,8) i; + create table gstest5_pg(id integer, v integer); + insert into gstest5_pg select i, i % 3 from generate_series(1,8) i; + """, + pg_conn, + ) + pg_conn.commit() + + for query in _PG19_GROUPING_SUBQUERY_QUERIES: + print(query) + # Single-element lists: assert_query_results_on_tables applies one + # string replace per entry, and "gstest5" is a substring of + # "gstest5_pg", so a duplicated entry would corrupt the heap query. + assert_query_results_on_tables(query, pg_conn, ["gstest5"], ["gstest5_pg"]) + + run_command("DROP TABLE IF EXISTS gstest5", pg_conn) + run_command("DROP TABLE IF EXISTS gstest5_pg", pg_conn) + pg_conn.commit() diff --git a/pg_lake_table/tests/pytests/test_pg19_new_features.py b/pg_lake_table/tests/pytests/test_pg19_new_features.py new file mode 100644 index 000000000..2a14b7a2b --- /dev/null +++ b/pg_lake_table/tests/pytests/test_pg19_new_features.py @@ -0,0 +1,911 @@ +"""Regression tests for PostgreSQL 19 commands/features interacting with pg_lake. + +PostgreSQL 19 introduces new statements that target regular tables (e.g. +REPACK). pg_lake managed iceberg tables (CREATE TABLE ... USING iceberg) and +pg_lake foreign tables are backed by the pg_lake FDW, so these heap-rewrite +commands must either be rejected with a meaningful error or be a harmless +no-op that does not corrupt the table. These tests pin that behaviour so a +future PostgreSQL or pg_lake change cannot silently regress it. + +Gated to PG19+ because the statements do not parse on older servers. +""" + +import pytest +from utils_pytest import * + +PG19 = 190000 + + +def _skip_if_not_pg19(conn): + if get_pg_version_num(conn) < PG19: + pytest.skip("PG19-only commands require PostgreSQL 19+") + + +def _make_parquet_foreign_table(conn, name, select_sql, columns_sql): + """Materialise `select_sql` into a Parquet file and expose it as a pg_lake + foreign table `name` with the given column definition. Returns the URL.""" + url = f"s3://{TEST_BUCKET}/test_pg19_features/{name}.parquet" + run_command(f"COPY ({select_sql}) TO '{url}' WITH (format 'parquet')", conn) + run_command(f"DROP FOREIGN TABLE IF EXISTS {name}", conn) + run_command( + f""" + CREATE FOREIGN TABLE {name} ({columns_sql}) + SERVER pg_lake OPTIONS (format 'parquet', path '{url}') + """, + conn, + ) + conn.commit() + return url + + +def _assert_rejected_or_harmless(conn, relation, repack_sql, expected_count): + """REPACK must either raise a meaningful error or leave the data intact. + + pg_lake tables are foreign tables under the hood, so PostgreSQL core is + expected to reject REPACK on them. We accept a clean error *or* a no-op + that preserves the row count; we reject silent data loss/corruption and + crashes. + """ + error = run_command(repack_sql, conn, raise_error=False) + conn.rollback() + + if error: + # Must be a clean, intelligible error -- not an internal crash. + lowered = error.lower() + assert any( + kw in lowered + for kw in ( + "foreign table", + "not a table", + "cannot", + "not supported", + "is not", + ) + ), f"REPACK on {relation} raised an unclear error: {error!r}" + else: + # If it "succeeded", the table must still be readable and intact. + rows = run_query(f"SELECT count(*) AS n FROM {relation}", conn) + conn.commit() + assert ( + int(rows[0]["n"]) == expected_count + ), f"REPACK on {relation} changed the row count unexpectedly" + + # The server must still be alive and the table still queryable. + rows = run_query(f"SELECT count(*) AS n FROM {relation}", conn) + conn.commit() + assert int(rows[0]["n"]) == expected_count + + +def test_repack_managed_iceberg_table(pg_conn, s3, extension, with_default_location): + """REPACK on a managed iceberg table must not corrupt it or crash PG.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_repack_iceberg", pg_conn) + run_command( + "CREATE TABLE pg19_repack_iceberg (id int) USING iceberg", + pg_conn, + ) + run_command( + "INSERT INTO pg19_repack_iceberg SELECT generate_series(1, 100)", + pg_conn, + ) + pg_conn.commit() + + _assert_rejected_or_harmless( + pg_conn, "pg19_repack_iceberg", "REPACK pg19_repack_iceberg", 100 + ) + + run_command("DROP TABLE IF EXISTS pg19_repack_iceberg", pg_conn) + pg_conn.commit() + + +def test_repack_foreign_table(pg_conn, s3, extension): + """REPACK on a pg_lake foreign table must not corrupt it or crash PG.""" + _skip_if_not_pg19(pg_conn) + + url = f"s3://{TEST_BUCKET}/test_pg19_repack/data.parquet" + run_command( + f"COPY (SELECT s AS id FROM generate_series(1, 100) s) TO '{url}'", + pg_conn, + ) + run_command("DROP FOREIGN TABLE IF EXISTS pg19_repack_fdw", pg_conn) + run_command( + f""" + CREATE FOREIGN TABLE pg19_repack_fdw (id int) + SERVER pg_lake OPTIONS (format 'parquet', path '{url}') + """, + pg_conn, + ) + pg_conn.commit() + + _assert_rejected_or_harmless( + pg_conn, "pg19_repack_fdw", "REPACK pg19_repack_fdw", 100 + ) + + run_command("DROP FOREIGN TABLE IF EXISTS pg19_repack_fdw", pg_conn) + pg_conn.commit() + + +def test_group_by_all_managed_iceberg(pg_conn, s3, extension, with_default_location): + """PG19 GROUP BY ALL must work transparently over a managed iceberg table.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_group_by_all", pg_conn) + run_command( + "CREATE TABLE pg19_group_by_all (g int, v int) USING iceberg", + pg_conn, + ) + # 3 groups (g = 0,1,2), 10 rows each. + run_command( + """ + INSERT INTO pg19_group_by_all + SELECT s % 3, s FROM generate_series(1, 30) s + """, + pg_conn, + ) + pg_conn.commit() + + rows = run_query( + "SELECT g, count(*) AS n FROM pg19_group_by_all GROUP BY ALL ORDER BY g", + pg_conn, + ) + pg_conn.commit() + + assert [(int(r["g"]), int(r["n"])) for r in rows] == [(0, 10), (1, 10), (2, 10)] + + run_command("DROP TABLE IF EXISTS pg19_group_by_all", pg_conn) + pg_conn.commit() + + +# GROUP BY ALL shapes lifted from the PG19 commit ef38a4d97 regression suite +# (src/test/regress/sql/aggregates.sql). Each must produce identical results on +# a pg_lake iceberg table and an identical heap table. The risk: deparsing +# re-emits the literal "GROUP BY ALL" rather than the inferred column list, so a +# pushed-down query would let DuckDB re-infer the grouping with a different +# algorithm -- a silent wrong result. pg_lake therefore refuses to push down +# GROUP BY ALL (see NOT_SHIPPABLE_SQL_GROUP_BY_ALL); these shapes pin both the +# local-execution fallback and that the results stay correct. +_GROUP_BY_ALL_SHAPES = [ + # basic single grouping column + "SELECT b, COUNT(*) AS n FROM {t} GROUP BY ALL", + # multiple grouping columns in non-consecutive target positions + "SELECT a, SUM(b) AS s, b FROM {t} GROUP BY ALL", + # grouping key is an expression, not a plain column + "SELECT a + b AS g FROM {t} GROUP BY ALL", + # the documentation example: mix of columns, an expression, and an aggregate + "SELECT a, b, a + b AS ab, sum(c) AS s FROM {t} GROUP BY ALL", + # aggregate nested in a larger expression -> only {a} is a grouping key + "SELECT a, SUM(b) + 4 AS s FROM {t} GROUP BY ALL", + # grouped column referenced inside the aggregate expression + "SELECT a, SUM(b) + a AS s FROM {t} GROUP BY ALL", + # all targets are aggregates -> reduces to GROUP BY () + "SELECT COUNT(a) AS ca, SUM(b) AS s FROM {t} GROUP BY ALL", + # star expansion: every column becomes a grouping key + "SELECT *, count(*) AS n FROM {t} GROUP BY ALL", +] + +# Window functions must be excluded from the inferred grouping list, just like +# aggregates. +_GROUP_BY_ALL_WINDOW_SHAPE = ( + "SELECT a, COUNT(a) OVER (PARTITION BY a) AS w FROM {t} GROUP BY ALL" +) + + +def test_group_by_all_correctness_and_no_pushdown( + pg_conn, s3, extension, with_default_location +): + """PG19 GROUP BY ALL over a managed iceberg table must produce exactly the + same results as an identical heap table for every grouping shape. + + PostgreSQL and DuckDB infer the grouping columns from the target list with + different algorithms, so pg_lake refuses to push GROUP BY ALL down to DuckDB + and runs it locally instead. We assert both: results match the heap baseline + *and* the query is not pushed down (so the not-shippable rule keeps firing).""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS gba_lake", pg_conn) + run_command("DROP TABLE IF EXISTS gba_heap", pg_conn) + run_command("CREATE TABLE gba_lake (a int, b int, c int) USING iceberg", pg_conn) + run_command("CREATE TABLE gba_heap (a int, b int, c int)", pg_conn) + # Data chosen so that incorrectly inferred grouping (e.g. grouping by a,b instead of + # (a+b), or failing to collapse to GROUP BY ()) yields different results. + data = """ + INSERT INTO {t} (a, b, c) VALUES + (1, 10, 100), (1, 10, 200), (1, 20, 300), + (2, 10, 400), (2, NULL, 500), (NULL, 30, 600), + (3, 7, 700), (3, 7, 800) + """ + run_command(data.format(t="gba_lake"), pg_conn) + run_command(data.format(t="gba_heap"), pg_conn) + pg_conn.commit() + + for shape in _GROUP_BY_ALL_SHAPES + [_GROUP_BY_ALL_WINDOW_SHAPE]: + query = shape.format(t="gba_lake") + # Correctness: iceberg result must equal the heap baseline (true PG + # GROUP BY ALL semantics). + assert_query_results_on_tables(query, pg_conn, ["gba_lake"], ["gba_heap"]) + # GROUP BY ALL must run locally, never be handed to DuckDB. + assert_query_not_pushdownable( + query, pg_conn, f"GROUP BY ALL must not be pushed down: {query}" + ) + + # The ALL/DISTINCT *grouping-set modifier* (e.g. "GROUP BY ALL a") is a + # different, older feature than the PG19 column-inference "GROUP BY ALL": + # here ALL is just the (default) duplicate-grouping-set modifier, so the + # grouping is the explicit "a" and the query stays pushdownable. Pin that we + # did not over-block it, and that it still produces correct results. + modifier_query = "SELECT a, count(*) AS n FROM gba_lake GROUP BY ALL a" + assert_query_results_on_tables(modifier_query, pg_conn, ["gba_lake"], ["gba_heap"]) + assert_query_pushdownable( + modifier_query, + pg_conn, + "GROUP BY with an explicit element must still push down", + ) + + # Error parity: a column that is neither grouped nor aggregated must be + # rejected at parse-analysis (before pushdown), identically to core. + error = run_command( + "SELECT a, SUM(b) + c AS s FROM gba_lake GROUP BY ALL", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert ( + error and "group by" in error.lower() + ), f"GROUP BY ALL with an ungrouped column should be rejected, got: {error!r}" + + run_command("DROP TABLE IF EXISTS gba_lake", pg_conn) + run_command("DROP TABLE IF EXISTS gba_heap", pg_conn) + pg_conn.commit() + + +# PG19 added IGNORE NULLS / RESPECT NULLS null-treatment to window functions +# (lead/lag/first_value/last_value/nth_value). pg_get_querydef (core PG19) emits +# the treatment *outside* the argument parens ("lag(v) IGNORE NULLS OVER ..."), +# so pushing such a query to DuckDB only yields correct results if DuckDB both +# parses that spelling and applies the same null skipping. These shapes pin +# correctness against a heap baseline; the data interleaves NULLs so that +# IGNORE vs RESPECT produce visibly different output. +# DuckDB's parser rejects the IGNORE NULLS spelling that core PG19 deparses, so +# pg_lake refuses to push these down (see the WindowFunc->ignore_nulls guard in +# query_pushdown.c) and runs them locally. Each must still match a heap baseline; +# the data interleaves NULLs so IGNORE vs RESPECT produce visibly different rows. +_IGNORE_NULLS_SHAPES = [ + "SELECT o, lag(v) IGNORE NULLS OVER w AS r FROM {t} WINDOW w AS (ORDER BY o)", + "SELECT o, lead(v) IGNORE NULLS OVER w AS r FROM {t} WINDOW w AS (ORDER BY o)", + "SELECT o, first_value(v) IGNORE NULLS OVER w AS r FROM {t} " + "WINDOW w AS (ORDER BY o ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)", + "SELECT o, last_value(v) IGNORE NULLS OVER w AS r FROM {t} " + "WINDOW w AS (ORDER BY o ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)", + "SELECT o, nth_value(v, 2) IGNORE NULLS OVER w AS r FROM {t} " + "WINDOW w AS (ORDER BY o ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)", +] + +# RESPECT NULLS is the default; parse analysis collapses it to NO_NULLTREATMENT, +# so it carries no clause and is pushed down to DuckDB normally. +_RESPECT_NULLS_SHAPE = ( + "SELECT o, lag(v) RESPECT NULLS OVER w AS r FROM {t} WINDOW w AS (ORDER BY o)" +) + + +def test_window_ignore_nulls_correctness(pg_conn, s3, extension, with_default_location): + """PG19 window IGNORE NULLS over a managed iceberg table must match an + identical heap table. DuckDB cannot parse core PG19's deparsed IGNORE NULLS + spelling, so pg_lake runs these locally; assert both correctness and that + they are not pushed down (so the not-shippable guard keeps firing). The + default RESPECT NULLS form still pushes down and must stay correct.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS wn_lake", pg_conn) + run_command("DROP TABLE IF EXISTS wn_heap", pg_conn) + run_command("CREATE TABLE wn_lake (o int, v int) USING iceberg", pg_conn) + run_command("CREATE TABLE wn_heap (o int, v int)", pg_conn) + data = """ + INSERT INTO {t} (o, v) VALUES + (1, 10), (2, NULL), (3, 30), (4, NULL), (5, NULL), + (6, 60), (7, NULL), (8, 80) + """ + run_command(data.format(t="wn_lake"), pg_conn) + run_command(data.format(t="wn_heap"), pg_conn) + pg_conn.commit() + + for shape in _IGNORE_NULLS_SHAPES: + query = shape.format(t="wn_lake") + assert_query_results_on_tables(query, pg_conn, ["wn_lake"], ["wn_heap"]) + assert_query_not_pushdownable( + query, pg_conn, f"IGNORE NULLS must not be pushed down: {query}" + ) + + respect_query = _RESPECT_NULLS_SHAPE.format(t="wn_lake") + assert_query_results_on_tables(respect_query, pg_conn, ["wn_lake"], ["wn_heap"]) + + run_command("DROP TABLE IF EXISTS wn_lake", pg_conn) + run_command("DROP TABLE IF EXISTS wn_heap", pg_conn) + pg_conn.commit() + + +def test_check_constraint_enforced_default_iceberg( + pg_conn, s3, extension, with_default_location +): + """A CHECK constraint on a managed iceberg table is ENFORCED by default + (PG19 makes ENFORCED explicit): a row violating it must be rejected, a + conforming row must be accepted.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_chk", pg_conn) + run_command( + "CREATE TABLE pg19_chk (a int, b int CHECK (b > 0)) USING iceberg", + pg_conn, + ) + pg_conn.commit() + + err = run_command("INSERT INTO pg19_chk VALUES (1, -5)", pg_conn, raise_error=False) + pg_conn.rollback() + assert ( + err and "pg19_chk_b_check" in err + ), f"ENFORCED CHECK must reject the violating row, got: {err!r}" + + run_command("INSERT INTO pg19_chk VALUES (1, 5)", pg_conn) + pg_conn.commit() + rows = run_query("SELECT count(*) AS n FROM pg19_chk", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 1 + + run_command("DROP TABLE IF EXISTS pg19_chk", pg_conn) + pg_conn.commit() + + +def test_check_constraint_not_enforced_iceberg( + pg_conn, s3, extension, with_default_location +): + """PG19 allows a CHECK constraint to be declared NOT ENFORCED at table + creation. On a managed iceberg table such a constraint must NOT reject + violating rows (core ExecRelCheck skips unenforced checks, and pg_lake + delegates enforcement to it).""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_chk_ne", pg_conn) + run_command( + "CREATE TABLE pg19_chk_ne (a int, b int, " + "CONSTRAINT b_pos CHECK (b > 0) NOT ENFORCED) USING iceberg", + pg_conn, + ) + pg_conn.commit() + + # NOT ENFORCED: the violating row is accepted, unlike the enforced case. + run_command("INSERT INTO pg19_chk_ne VALUES (1, -5)", pg_conn) + pg_conn.commit() + rows = run_query("SELECT count(*) AS n FROM pg19_chk_ne", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 1 + + # Managed iceberg tables are foreign-table backed, so core PG rejects the + # PG19 ALTER CONSTRAINT ... ENFORCED toggle outright. Pin that this is a + # clean rejection (not a crash or silent no-op that would change semantics). + enforce_err = run_command( + "ALTER TABLE pg19_chk_ne ALTER CONSTRAINT b_pos ENFORCED", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert ( + enforce_err and "alter constraint" in enforce_err.lower() + ), f"ALTER CONSTRAINT ENFORCED on iceberg should be cleanly rejected, got: {enforce_err!r}" + + run_command("DROP TABLE IF EXISTS pg19_chk_ne", pg_conn) + pg_conn.commit() + + +def test_set_expression_rejected_iceberg(pg_conn, s3, extension, with_default_location): + """ALTER COLUMN ... SET EXPRESSION (PG19 extends this to virtual generated + columns) is not supported on managed iceberg tables and must be rejected + cleanly rather than crash or silently no-op.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_setexpr", pg_conn) + run_command( + "CREATE TABLE pg19_setexpr (a int, g int GENERATED ALWAYS AS (a * 2) STORED) " + "USING iceberg", + pg_conn, + ) + pg_conn.commit() + + err = run_command( + "ALTER TABLE pg19_setexpr ALTER COLUMN g SET EXPRESSION AS (a * 3)", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert err, "SET EXPRESSION on an iceberg column should be rejected" + lowered = err.lower() + assert ( + "set expression" in lowered or "not supported" in lowered or "cannot" in lowered + ), f"SET EXPRESSION rejection should be intelligible, got: {err!r}" + + run_command("DROP TABLE IF EXISTS pg19_setexpr", pg_conn) + pg_conn.commit() + + +def test_on_conflict_do_select_managed_iceberg( + pg_conn, s3, extension, with_default_location +): + """PG19 INSERT ... ON CONFLICT DO SELECT must error meaningfully (no unique + index support on managed iceberg tables) rather than crash.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_on_conflict", pg_conn) + run_command( + "CREATE TABLE pg19_on_conflict (id int, v text) USING iceberg", + pg_conn, + ) + run_command("INSERT INTO pg19_on_conflict VALUES (1, 'a')", pg_conn) + pg_conn.commit() + + # DO SELECT requires a RETURNING clause; include it so we actually reach + # the conflict-arbiter resolution against the managed iceberg table. + error = run_command( + """ + INSERT INTO pg19_on_conflict VALUES (1, 'b') + ON CONFLICT (id) DO SELECT RETURNING * + """, + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + + # Managed iceberg tables have no unique index / arbiter, so this must be + # rejected with a clear error, not an internal failure. + assert error, "Expected ON CONFLICT DO SELECT to be rejected" + lowered = error.lower() + assert any( + kw in lowered + for kw in ( + "no unique", + "constraint", + "on conflict", + "not supported", + "cannot", + "unique or exclusion", + "arbiter", + ) + ), f"ON CONFLICT DO SELECT raised an unclear error: {error!r}" + + # Server still alive and table intact. + rows = run_query("SELECT count(*) AS n FROM pg19_on_conflict", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 1 + + run_command("DROP TABLE IF EXISTS pg19_on_conflict", pg_conn) + pg_conn.commit() + + +def test_repack_concurrently_managed_iceberg( + pg_conn, s3, extension, with_default_location +): + """PG19 also adds REPACK CONCURRENTLY; it must be handled as cleanly as the + plain form on a managed iceberg table (foreign table under the hood).""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_repack_conc", pg_conn) + run_command("CREATE TABLE pg19_repack_conc (id int) USING iceberg", pg_conn) + run_command("INSERT INTO pg19_repack_conc SELECT generate_series(1, 50)", pg_conn) + pg_conn.commit() + + _assert_rejected_or_harmless( + pg_conn, + "pg19_repack_conc", + "REPACK (CONCURRENTLY) pg19_repack_conc", + 50, + ) + + run_command("DROP TABLE IF EXISTS pg19_repack_conc", pg_conn) + pg_conn.commit() + + +def test_on_conflict_do_select_foreign_table(pg_conn, s3, extension): + """ON CONFLICT DO SELECT against a read-backed pg_lake foreign table must be + rejected with a clear error (no arbiter index), not an internal failure.""" + _skip_if_not_pg19(pg_conn) + + _make_parquet_foreign_table( + pg_conn, + "pg19_oc_ft", + "SELECT s AS id, 'v' || s AS v FROM generate_series(1, 5) s", + "id int, v text", + ) + + error = run_command( + """ + INSERT INTO pg19_oc_ft VALUES (1, 'dup') + ON CONFLICT (id) DO SELECT RETURNING * + """, + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + + assert error, "Expected ON CONFLICT DO SELECT on a foreign table to be rejected" + lowered = error.lower() + assert any( + kw in lowered + for kw in ( + "no unique", + "constraint", + "on conflict", + "not supported", + "cannot", + "unique or exclusion", + "arbiter", + "foreign table", + ) + ), f"ON CONFLICT DO SELECT on foreign table raised an unclear error: {error!r}" + + run_command("DROP FOREIGN TABLE IF EXISTS pg19_oc_ft", pg_conn) + pg_conn.commit() + + +def test_group_by_all_foreign_table(pg_conn, s3, extension): + """PG19 GROUP BY ALL must work over a pg_lake foreign table, including a + HAVING clause and a non-trivial aggregate, matching explicit GROUP BY.""" + _skip_if_not_pg19(pg_conn) + + # 4 groups (g = 0..3), with a running value column. + _make_parquet_foreign_table( + pg_conn, + "pg19_gba_ft", + "SELECT (s % 4) AS g, s AS v FROM generate_series(1, 40) s", + "g int, v int", + ) + + explicit = run_query( + """ + SELECT g, count(*) AS n, sum(v) AS s FROM pg19_gba_ft + GROUP BY g HAVING count(*) > 1 ORDER BY g + """, + pg_conn, + ) + pg_conn.commit() + + group_by_all = run_query( + """ + SELECT g, count(*) AS n, sum(v) AS s FROM pg19_gba_ft + GROUP BY ALL HAVING count(*) > 1 ORDER BY g + """, + pg_conn, + ) + pg_conn.commit() + + assert group_by_all == explicit, "GROUP BY ALL diverged from explicit GROUP BY" + assert len(group_by_all) == 4 + + run_command("DROP FOREIGN TABLE IF EXISTS pg19_gba_ft", pg_conn) + pg_conn.commit() + + +def test_group_by_all_partitioned_lake(pg_conn, s3, extension): + """GROUP BY ALL must work transparently across a declaratively partitioned + table whose partitions are pg_lake foreign tables.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_gba_parent CASCADE", pg_conn) + run_command( + "CREATE TABLE pg19_gba_parent (id int, g int) PARTITION BY RANGE (id)", + pg_conn, + ) + pg_conn.commit() + + for part in range(2): + lo, hi = part * 100, (part + 1) * 100 + url = f"s3://{TEST_BUCKET}/test_pg19_features/gba_part{part}.parquet" + run_command( + f""" + COPY (SELECT s AS id, (s % 3) AS g + FROM generate_series({lo}, {hi - 1}) s) + TO '{url}' WITH (format 'parquet') + """, + pg_conn, + ) + run_command( + f""" + CREATE FOREIGN TABLE pg19_gba_part{part} + PARTITION OF pg19_gba_parent FOR VALUES FROM ({lo}) TO ({hi}) + SERVER pg_lake OPTIONS (format 'parquet', path '{url}') + """, + pg_conn, + ) + pg_conn.commit() + + rows = run_query( + "SELECT g, count(*) AS n FROM pg19_gba_parent GROUP BY ALL ORDER BY g", + pg_conn, + ) + pg_conn.commit() + + # 200 rows over g = id % 3 -> groups 0,1,2. + counts = {int(r["g"]): int(r["n"]) for r in rows} + assert sum(counts.values()) == 200 + assert set(counts.keys()) == {0, 1, 2} + + run_command("DROP TABLE IF EXISTS pg19_gba_parent CASCADE", pg_conn) + pg_conn.commit() + + +def test_copy_to_partitioned_parent_direct(pg_conn, s3, extension): + """PG19 (commit 4bea91f21) lets COPY a partitioned table directly. pg_lake + now supports this for lake-format COPY too: COPY TO descends into + the partitions (parent holds no rows) and round-trips. The legacy + COPY (SELECT ...) form must keep working as well. + """ + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_copy_part CASCADE", pg_conn) + run_command("DROP TABLE IF EXISTS pg19_copy_part_rt", pg_conn) + run_command( + """ + CREATE TABLE pg19_copy_part (id int, v text) PARTITION BY RANGE (id); + CREATE TABLE pg19_copy_part_1 PARTITION OF pg19_copy_part + FOR VALUES FROM (0) TO (100); + CREATE TABLE pg19_copy_part_2 PARTITION OF pg19_copy_part + FOR VALUES FROM (100) TO (200); + INSERT INTO pg19_copy_part SELECT g, 'v' || g FROM generate_series(0, 199) g; + """, + pg_conn, + ) + pg_conn.commit() + + url = f"s3://{TEST_BUCKET}/test_pg19_features/copy_part.parquet" + + # Direct COPY TO now succeeds on PG19. + run_command( + f"COPY pg19_copy_part TO '{url}' WITH (format 'parquet')", + pg_conn, + ) + pg_conn.commit() + + run_command("CREATE TABLE pg19_copy_part_rt (id int, v text)", pg_conn) + run_command(f"COPY pg19_copy_part_rt FROM '{url}' WITH (format 'parquet')", pg_conn) + pg_conn.commit() + + rows = run_query("SELECT count(*) AS n FROM pg19_copy_part_rt", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 200 + + # The legacy COPY (SELECT ...) TO form must keep working as well. + run_command("TRUNCATE pg19_copy_part_rt", pg_conn) + run_command( + f"COPY (SELECT * FROM pg19_copy_part) TO '{url}' WITH (format 'parquet')", + pg_conn, + ) + run_command(f"COPY pg19_copy_part_rt FROM '{url}' WITH (format 'parquet')", pg_conn) + pg_conn.commit() + rows = run_query("SELECT count(*) AS n FROM pg19_copy_part_rt", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 200 + + run_command("DROP TABLE IF EXISTS pg19_copy_part CASCADE", pg_conn) + run_command("DROP TABLE IF EXISTS pg19_copy_part_rt", pg_conn) + pg_conn.commit() + + +def test_merge_partitions_with_lake_partitions(pg_conn, s3, extension): + """PG19 ALTER TABLE ... MERGE PARTITIONS cannot merge pg_lake foreign-table + partitions: core rejects them because a foreign table "is not a table". + Pin that exact rejection and that the data survives unchanged.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_merge_parent CASCADE", pg_conn) + run_command( + "CREATE TABLE pg19_merge_parent (id int) PARTITION BY RANGE (id)", pg_conn + ) + pg_conn.commit() + + for part in range(2): + lo, hi = part * 100, (part + 1) * 100 + url = f"s3://{TEST_BUCKET}/test_pg19_features/merge_part{part}.parquet" + run_command( + f"COPY (SELECT generate_series({lo}, {hi - 1}) AS id) " + f"TO '{url}' WITH (format 'parquet')", + pg_conn, + ) + run_command( + f""" + CREATE FOREIGN TABLE pg19_merge_part{part} + PARTITION OF pg19_merge_parent FOR VALUES FROM ({lo}) TO ({hi}) + SERVER pg_lake OPTIONS (format 'parquet', path '{url}') + """, + pg_conn, + ) + pg_conn.commit() + + error = run_command( + """ + ALTER TABLE pg19_merge_parent + MERGE PARTITIONS (pg19_merge_part0, pg19_merge_part1) INTO pg19_merge_merged + """, + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + + assert error and "is not a table" in error, ( + f"MERGE PARTITIONS over a foreign-table partition must be rejected with " + f'"is not a table", got: {error!r}' + ) + + # The failed ALTER must leave the existing partitions and their data intact. + rows = run_query("SELECT count(*) AS n FROM pg19_merge_parent", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 200 + + run_command("DROP TABLE IF EXISTS pg19_merge_parent CASCADE", pg_conn) + pg_conn.commit() + + +def test_split_partition_with_lake_partition(pg_conn, s3, extension): + """PG19 ALTER TABLE ... SPLIT PARTITION cannot split a pg_lake foreign-table + partition: core rejects it because a foreign table "is not a table". Pin + that exact rejection and that the data survives unchanged.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_split_parent CASCADE", pg_conn) + run_command( + "CREATE TABLE pg19_split_parent (id int) PARTITION BY RANGE (id)", pg_conn + ) + url = f"s3://{TEST_BUCKET}/test_pg19_features/split_part.parquet" + run_command( + f"COPY (SELECT generate_series(0, 199) AS id) " + f"TO '{url}' WITH (format 'parquet')", + pg_conn, + ) + run_command( + f""" + CREATE FOREIGN TABLE pg19_split_part + PARTITION OF pg19_split_parent FOR VALUES FROM (0) TO (200) + SERVER pg_lake OPTIONS (format 'parquet', path '{url}') + """, + pg_conn, + ) + pg_conn.commit() + + error = run_command( + """ + ALTER TABLE pg19_split_parent SPLIT PARTITION pg19_split_part INTO ( + PARTITION pg19_split_a FOR VALUES FROM (0) TO (100), + PARTITION pg19_split_b FOR VALUES FROM (100) TO (200) + ) + """, + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + + assert error and "is not a table" in error, ( + f"SPLIT PARTITION over a foreign-table partition must be rejected with " + f'"is not a table", got: {error!r}' + ) + + # The failed ALTER must leave the existing partition and its data intact. + rows = run_query("SELECT count(*) AS n FROM pg19_split_parent", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 200 + + run_command("DROP TABLE IF EXISTS pg19_split_parent CASCADE", pg_conn) + pg_conn.commit() + + +def test_on_conflict_do_select_for_update_managed_iceberg( + pg_conn, s3, extension, with_default_location +): + """PG19 also allows the locking clause on INSERT ... ON CONFLICT DO SELECT + (e.g. DO SELECT FOR UPDATE). Managed iceberg tables have no unique index to + serve as a conflict arbiter, so this must be rejected with a clear error + rather than crash, exactly like the non-locking DO SELECT form.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_ocfu", pg_conn) + run_command("CREATE TABLE pg19_ocfu (id int, v text) USING iceberg", pg_conn) + run_command("INSERT INTO pg19_ocfu VALUES (1, 'a')", pg_conn) + pg_conn.commit() + + error = run_command( + """ + INSERT INTO pg19_ocfu VALUES (1, 'b') + ON CONFLICT (id) DO SELECT FOR UPDATE RETURNING * + """, + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + + assert error, "Expected ON CONFLICT DO SELECT FOR UPDATE to be rejected" + lowered = error.lower() + assert any( + kw in lowered + for kw in ("no unique", "unique or exclusion", "arbiter", "constraint") + ), f"ON CONFLICT DO SELECT FOR UPDATE raised an unclear error: {error!r}" + + rows = run_query("SELECT count(*) AS n FROM pg19_ocfu", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 1 + + run_command("DROP TABLE IF EXISTS pg19_ocfu", pg_conn) + pg_conn.commit() + + +def test_update_for_portion_of_rejected_iceberg( + pg_conn, s3, extension, with_default_location +): + """PG19 adds UPDATE/DELETE ... FOR PORTION OF for temporal range columns. + pg_lake tables are foreign-table backed, and core PG explicitly rejects + FOR PORTION OF on foreign tables. Pin that both UPDATE and DELETE forms are + cleanly rejected and leave the data untouched.""" + _skip_if_not_pg19(pg_conn) + + run_command("DROP TABLE IF EXISTS pg19_portion", pg_conn) + run_command( + "CREATE TABLE pg19_portion (id int, valid int4range) USING iceberg", pg_conn + ) + run_command("INSERT INTO pg19_portion VALUES (1, '[1,10)')", pg_conn) + pg_conn.commit() + + update_err = run_command( + "UPDATE pg19_portion FOR PORTION OF valid FROM 3 TO 5 SET id = 2", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert update_err and "for portion of" in update_err.lower(), ( + f"UPDATE FOR PORTION OF on iceberg must be cleanly rejected, " + f"got: {update_err!r}" + ) + + delete_err = run_command( + "DELETE FROM pg19_portion FOR PORTION OF valid FROM 3 TO 5", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + assert delete_err and "for portion of" in delete_err.lower(), ( + f"DELETE FOR PORTION OF on iceberg must be cleanly rejected, " + f"got: {delete_err!r}" + ) + + rows = run_query("SELECT count(*) AS n FROM pg19_portion", pg_conn) + pg_conn.commit() + assert int(rows[0]["n"]) == 1 + + run_command("DROP TABLE IF EXISTS pg19_portion", pg_conn) + pg_conn.commit() + + +def test_property_graph_over_lake_table_rejected(pg_conn, s3, extension): + """PG19 adds SQL/PGQ property graphs and GRAPH_TABLE. Defining a property + graph requires every element table to have a key, but pg_lake foreign tables + cannot declare a primary key, so CREATE PROPERTY GRAPH over a pg_lake table + is cleanly rejected (no key). Pin that this is a clear error rather than a + crash, documenting that pg_lake tables are not usable as graph elements.""" + _skip_if_not_pg19(pg_conn) + + url = _make_parquet_foreign_table( + pg_conn, + "pg19_graph_ft", + "SELECT 1 AS id, 2 AS dst", + "id int, dst int", + ) + assert url + + error = run_command( + "CREATE PROPERTY GRAPH pg19_graph VERTEX TABLES (pg19_graph_ft)", + pg_conn, + raise_error=False, + ) + pg_conn.rollback() + + assert error, "Expected CREATE PROPERTY GRAPH over a pg_lake table to be rejected" + lowered = error.lower() + assert ( + "key" in lowered + ), f"CREATE PROPERTY GRAPH over a pg_lake table raised an unclear error: {error!r}" + + run_command("DROP FOREIGN TABLE IF EXISTS pg19_graph_ft CASCADE", pg_conn) + pg_conn.commit() diff --git a/pg_lake_table/tests/pytests/test_pgduck_server_crash.py b/pg_lake_table/tests/pytests/test_pgduck_server_crash.py index d4d5af688..2158aab71 100644 --- a/pg_lake_table/tests/pytests/test_pgduck_server_crash.py +++ b/pg_lake_table/tests/pytests/test_pgduck_server_crash.py @@ -156,18 +156,32 @@ def run_scan(): # socket; the next PQconsumeInput() call will return false. _kill_pgduck_server() - # Wake the backend. PQconsumeInput() will detect the broken - # connection and throw an ERROR. Without the fix this causes a - # double-free SIGSEGV that crashes the whole PostgreSQL cluster. + # Detach the injection point *before* waking the backend. + # + # The injection point fires on every iteration of the WaitForResult + # poll loop (while (PQisBusy(conn))), not just once. When the backend + # is woken it re-enters that loop: WaitLatchOrSocket() may return + # because MyLatch was set (by the wakeup/interrupt) before the killed + # socket's EOF is reported as readable, so PQconsumeInput() is not yet + # called and the loop iterates again. If the injection point were + # still armed at that moment the backend would pause on it a second + # time and the single wakeup below would already have been consumed, + # leaving the backend asleep forever (a detach does not wake an + # already-sleeping waiter). Detaching first guarantees the re-entered + # loop finds no injection point and proceeds to observe the broken + # connection. This race is timing-dependent and surfaces on PG19. run_command( - f"SELECT injection_points_wakeup('{INJECTION_POINT}')", + f"SELECT injection_points_detach('{INJECTION_POINT}')", superuser_conn, ) superuser_conn.commit() - # Detach so subsequent queries are not blocked. + # Wake the backend. It re-enters the (now injection-point-free) poll + # loop and PQconsumeInput() detects the broken connection and throws an + # ERROR. Without the WaitForResult fix this causes a double-free + # SIGSEGV that crashes the whole PostgreSQL cluster. run_command( - f"SELECT injection_points_detach('{INJECTION_POINT}')", + f"SELECT injection_points_wakeup('{INJECTION_POINT}')", superuser_conn, ) superuser_conn.commit() diff --git a/pg_lake_table/tests/sample_extensions/pg_lake_table_test_data_file_hook/Makefile b/pg_lake_table/tests/sample_extensions/pg_lake_table_test_data_file_hook/Makefile index c9fbbbac1..bd64cca60 100644 --- a/pg_lake_table/tests/sample_extensions/pg_lake_table_test_data_file_hook/Makefile +++ b/pg_lake_table/tests/sample_extensions/pg_lake_table_test_data_file_hook/Makefile @@ -6,7 +6,7 @@ DATA = $(EXTENSION)--1.0.sql SOURCES := $(wildcard src/*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -I../../../include -I../../../../pg_extension_base/include -I../../../../pg_lake_engine/include -I../../../../pg_lake_iceberg/include -I$(shell $(PG_CONFIG) --includedir) -std=gnu99 +PG_CPPFLAGS = -Iinclude -I../../../include -I../../../../pg_extension_base/include -I../../../../pg_lake_engine/include -I../../../../pg_lake_iceberg/include -I$(shell $(PG_CONFIG) --includedir) -std=gnu11 override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) PG_CONFIG ?= pg_config diff --git a/pg_lake_table/tests/sample_extensions/pg_lake_table_test_hideable/Makefile b/pg_lake_table/tests/sample_extensions/pg_lake_table_test_hideable/Makefile index 34073d357..0d8667fec 100644 --- a/pg_lake_table/tests/sample_extensions/pg_lake_table_test_hideable/Makefile +++ b/pg_lake_table/tests/sample_extensions/pg_lake_table_test_hideable/Makefile @@ -6,7 +6,7 @@ DATA = $(EXTENSION)--1.0.sql SOURCES := $(wildcard src/*.c) OBJS := $(patsubst %.c,%.o,$(sort $(SOURCES))) -PG_CPPFLAGS = -Iinclude -I../../../include -I../../../../pg_extension_base/include -std=gnu99 +PG_CPPFLAGS = -Iinclude -I../../../include -I../../../../pg_extension_base/include -std=gnu11 override CFLAGS := $(filter-out -Wdeclaration-after-statement,$(CFLAGS)) PG_CONFIG ?= pg_config diff --git a/pg_map/src/map.c b/pg_map/src/map.c index 15bf0daa3..90c8bd22a 100644 --- a/pg_map/src/map.c +++ b/pg_map/src/map.c @@ -53,6 +53,7 @@ #include "utils/regproc.h" #include "utils/syscache.h" #include "utils/typcache.h" +#include "utils/tuplestore.h" /* Check PgSQL version */ @@ -564,6 +565,14 @@ map_create_type(char *typeName, Oid keyElementType, Oid valElementType) mapTypeAddr = DefineDomain(newDomain); #endif + /* + * Make the new domain visible to syscache before we use its OID as a + * function argument type below. PG19's ProcedureCreate() walks each + * dependency through get_object_namespace() (find_temp_object); without + * this CCI the pg_type row is invisible and the lookup raises an error. + */ + CommandCounterIncrement(); + /* map pair depend on map to make sure they are always dropped together */ recordDependencyOn(&keyValPairTypeAddr, &mapTypeAddr, DEPENDENCY_NORMAL); diff --git a/pgduck_server/Makefile b/pgduck_server/Makefile index 5f6c956aa..3dd88b740 100644 --- a/pgduck_server/Makefile +++ b/pgduck_server/Makefile @@ -12,9 +12,11 @@ PGXS := $(shell $(PG_CONFIG) --pgxs) PG_LAKE_DELTA_SUPPORT ?= 0 -# compile with C11 option to use modern C +# compile with C11 option to use modern C; use GNU dialect so that +# typeof / static_assert resolve when including PG19's c.h, which +# uses both unconditionally # use duckdb.h from the duckdb submodule -PG_CPPFLAGS = -std=c11 -I$(PG_INCLUDEDIR) -Iinclude -DPG_LAKE_DELTA_SUPPORT=$(PG_LAKE_DELTA_SUPPORT) +PG_CPPFLAGS = -std=gnu11 -I$(PG_INCLUDEDIR) -Iinclude -DPG_LAKE_DELTA_SUPPORT=$(PG_LAKE_DELTA_SUPPORT) # Add dependencies SHLIB_LINK_INTERNAL = $(libpq) diff --git a/pgduck_server/include/utils/numutils.h b/pgduck_server/include/utils/numutils.h index daabe4dd8..eb349908c 100644 --- a/pgduck_server/include/utils/numutils.h +++ b/pgduck_server/include/utils/numutils.h @@ -25,8 +25,6 @@ #include "c.h" -#include "nodes/nodes.h" - extern int pg_itoa(int16 i, char *a); extern int pg_ulltoa_n(uint64 value, char *a); extern int pg_ultoa_n(uint32 value, char *a); diff --git a/pgduck_server/src/duckdb/type_conversion.c b/pgduck_server/src/duckdb/type_conversion.c index 6424abc43..4b7171288 100644 --- a/pgduck_server/src/duckdb/type_conversion.c +++ b/pgduck_server/src/duckdb/type_conversion.c @@ -22,6 +22,7 @@ * * Copyright (c) 2025 Snowflake Computing, Inc. All rights reserved. */ +#include "catalog/pg_type_d.h" #include "c.h" #include "postgres_fe.h" diff --git a/pgduck_server/src/pgsession/pgsession_io.c b/pgduck_server/src/pgsession/pgsession_io.c index 5e430f826..75737cd90 100644 --- a/pgduck_server/src/pgsession/pgsession_io.c +++ b/pgduck_server/src/pgsession/pgsession_io.c @@ -281,12 +281,77 @@ pgsession_read_startup_packet(PGSession * pgSession) pg_free(startupPacketBuf); return EOF; } - else if (proto != PG_PROTOCOL(3, 0)) + else if (PG_PROTOCOL_MAJOR(proto) != 3) { PGDUCK_SERVER_ERROR("unexpected protocol message: %u", proto); pg_free(startupPacketBuf); return EOF; } + else if (proto != PG_PROTOCOL(3, 0)) + { + /* + * Client asked for a 3.x version higher than 3.0 (e.g. PG19's libpq + * defaults to PG_PROTOCOL_GREASE = 3.9999 to probe servers that + * support NegotiateProtocolVersion). Reply with a 'v' + * (NegotiateProtocolVersion) message advertising 3.0 plus the names + * of any "_pq_.*" protocol options we did not recognize, mirroring + * what core PostgreSQL does in backend_startup.c. libpq verifies that + * every _pq_.* option it sent is echoed back; otherwise it aborts + * with "server did not report the unsupported ... parameter in its + * protocol negotiation message". + */ + StringInfoData buf; + int32 offset = sizeof(ProtocolVersion); + int32 numUnrecognized = 0; + + /* + * First pass: count "_pq_.*" options. The startup body is a series of + * NUL-terminated name/value pairs followed by a final empty name. We + * rely on the trailing NUL appended in the malloc above so that + * unterminated strings can't run past the buffer. + */ + while (offset < startupPacketLen) + { + const char *name = startupPacketBuf + offset; + int32 valOffset; + + if (*name == '\0') + break; + valOffset = offset + (int32) strlen(name) + 1; + if (valOffset >= startupPacketLen) + break; + if (strncmp(name, "_pq_.", 5) == 0) + numUnrecognized++; + offset = valOffset + (int32) strlen(startupPacketBuf + valOffset) + 1; + } + + pq_beginmessage(&buf, 'v'); + pq_sendint32(&buf, PG_PROTOCOL(3, 0)); + pq_sendint32(&buf, numUnrecognized); + + /* Second pass: emit the names. */ + offset = sizeof(ProtocolVersion); + while (offset < startupPacketLen) + { + const char *name = startupPacketBuf + offset; + int32 valOffset; + + if (*name == '\0') + break; + valOffset = offset + (int32) strlen(name) + 1; + if (valOffset >= startupPacketLen) + break; + if (strncmp(name, "_pq_.", 5) == 0) + pq_sendstring(&buf, name); + offset = valOffset + (int32) strlen(startupPacketBuf + valOffset) + 1; + } + + if (!IsOK(pq_endmessage(pgSession, &buf))) + { + pg_free(startupPacketBuf); + return EOF; + } + } pg_free(startupPacketBuf); return OK; diff --git a/pgduck_server/tests/ctests/Makefile b/pgduck_server/tests/ctests/Makefile index 630245b61..2f1519908 100644 --- a/pgduck_server/tests/ctests/Makefile +++ b/pgduck_server/tests/ctests/Makefile @@ -12,8 +12,10 @@ PG_INCLUDEDIR = $(shell $(PG_CONFIG) --includedir) # PGXS: Locates PostgreSQL extension building infrastructure using 'pg_config'. PGXS := $(shell $(PG_CONFIG) --pgxs) -# compile with C11 option to use modern C -PG_CPPFLAGS = -std=c11 -I$(PG_INCLUDEDIR) -g +# compile with C11 option to use modern C; use GNU dialect so that +# typeof / static_assert resolve when including PG19's c.h, which +# uses both unconditionally +PG_CPPFLAGS = -std=gnu11 -I$(PG_INCLUDEDIR) -g PG_LDFLAGS = -L$(PG_LIBDIR) -lpq -lpgcommon -lduckdb -Wl,-rpath,$(PG_LIBDIR) diff --git a/pgduck_server/tests/pytests/test_protocol_negotiation.py b/pgduck_server/tests/pytests/test_protocol_negotiation.py new file mode 100644 index 000000000..ffedb8dba --- /dev/null +++ b/pgduck_server/tests/pytests/test_protocol_negotiation.py @@ -0,0 +1,159 @@ +"""Wire-protocol coverage for pgduck_server's NegotiateProtocolVersion handling. + +PostgreSQL 18 introduced wire protocol 3.2 together with the +NegotiateProtocolVersion ('v') backend message, and PG18+ libpq probes servers +by requesting a higher minor version than it strictly needs +(PG_PROTOCOL_GREASE = 3.9999) and/or by sending forward-compatible "_pq_.*" +protocol options. A server that only speaks 3.0 -- like pgduck_server -- must: + + * accept the connection, + * reply with a 'v' message advertising the version it actually speaks (3.0), + * echo back the name of every "_pq_.*" option it did not recognize, + +otherwise modern libpq aborts with "server did not report the unsupported ... +parameter in its protocol negotiation message". + +These tests drive the raw wire protocol so they pin pgduck_server's behaviour +deterministically, independent of whatever libpq version happens to run the +suite. They also guard against regressing the plain-3.0 fast path (which must +NOT emit a 'v' message) and against a malformed higher-major request taking the +server down. +""" + +import socket +from pathlib import Path + +import pytest +from utils_pytest import * +from utils_protocol import * + + +def _connect(): + socket_path = str( + Path(server_params.PGDUCK_UNIX_DOMAIN_PATH) + / f".s.PGSQL.{server_params.PGDUCK_PORT}" + ) + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + s.connect(socket_path) + return s + + +def _assert_server_still_usable(): + run_simple_command(server_params.PGDUCK_UNIX_DOMAIN_PATH, server_params.PGDUCK_PORT) + + +def test_plain_3_0_startup_has_no_negotiation(pgduck_server): + """A vanilla 3.0 client must NOT receive a NegotiateProtocolVersion message; + the very first backend message is AuthenticationOk ('R').""" + with _connect() as s: + send_startup_message(s) # defaults to protocol 3.0 + msg_type, _ = read_backend_message(s) + assert ( + msg_type == "R" + ), f"expected AuthenticationOk first on a 3.0 connection, got {msg_type!r}" + # And the session reaches ReadyForQuery as usual. + read_until(s, "Z") + _assert_server_still_usable() + + +@pytest.mark.parametrize( + "protocol_version", + [PG_PROTOCOL_3_2, PG_PROTOCOL_GREASE], + ids=["minor_3_2", "grease_3_9999"], +) +def test_negotiate_downgrades_to_3_0(pgduck_server, protocol_version): + """A client requesting a 3.x minor version > 0 receives a 'v' that advertises + exactly 3.0 with no unrecognized options, then a fully usable session.""" + with _connect() as s: + send_startup_message(s, protocol_version=protocol_version) + + msg_type, payload = read_backend_message(s) + assert ( + msg_type == "v" + ), f"expected NegotiateProtocolVersion for {protocol_version}, got {msg_type!r}" + version, names = parse_negotiate_protocol_version(payload) + assert version == PG_PROTOCOL_3_0, f"server advertised {version}, expected 3.0" + assert names == [], f"unexpected unrecognized options: {names}" + + # Startup must still complete after negotiation. + read_until(s, "Z") + + # ... and the negotiated session must actually execute queries. + send_query_message(s, "SELECT 1") + types = [m[0] for m in read_until(s, "Z")] + assert "C" in types, f"SELECT 1 did not complete (saw {types})" + _assert_server_still_usable() + + +def test_negotiate_without_pq_options_reports_zero(pgduck_server): + """A higher minor version carrying only ordinary parameters yields a 'v' + message with a zero unrecognized-option count.""" + with _connect() as s: + send_startup_message( + s, + protocol_version=PG_PROTOCOL_3_2, + params={"user": "postgres", "application_name": "pg19-protocol-test"}, + ) + msg_type, payload = read_backend_message(s) + assert msg_type == "v", f"expected NegotiateProtocolVersion, got {msg_type!r}" + version, names = parse_negotiate_protocol_version(payload) + assert version == PG_PROTOCOL_3_0 + assert names == [] + read_until(s, "Z") + _assert_server_still_usable() + + +def test_negotiate_echoes_unrecognized_pq_options(pgduck_server): + """Every "_pq_.*" option the client sends must be echoed back verbatim in the + 'v' message (libpq aborts otherwise), while ordinary parameters are not.""" + params = { + "_pq_.protocol_managed_a": "1", + "user": "postgres", + "_pq_.protocol_managed_b": "on", + "database": "regression", + } + with _connect() as s: + send_startup_message(s, protocol_version=PG_PROTOCOL_GREASE, params=params) + + msg_type, payload = read_backend_message(s) + assert msg_type == "v", f"expected NegotiateProtocolVersion, got {msg_type!r}" + version, names = parse_negotiate_protocol_version(payload) + assert version == PG_PROTOCOL_3_0 + assert set(names) == { + "_pq_.protocol_managed_a", + "_pq_.protocol_managed_b", + }, f"echoed option names were {names}" + + read_until(s, "Z") + _assert_server_still_usable() + + +def test_negotiate_echoes_single_pq_option(pgduck_server): + """The common libpq case: a single unrecognized _pq_ option at GREASE.""" + with _connect() as s: + send_startup_message( + s, + protocol_version=PG_PROTOCOL_GREASE, + params={"_pq_.report_meaningless_option": "1", "user": "postgres"}, + ) + msg_type, payload = read_backend_message(s) + assert msg_type == "v" + version, names = parse_negotiate_protocol_version(payload) + assert version == PG_PROTOCOL_3_0 + assert names == ["_pq_.report_meaningless_option"] + read_until(s, "Z") + _assert_server_still_usable() + + +def test_unsupported_major_version_rejected(pgduck_server): + """A non-3 major protocol version must be rejected and the connection closed, + without ever emitting a 'v'/auth message and without taking the server down.""" + # Repeat so a failure can't leave the listener in a bad state for others. + for _ in range(3): + with _connect() as s: + send_startup_message(s, protocol_version=PG_PROTOCOL_4_0) + data = s.recv(1024) + assert ( + data == b"" + ), f"expected connection close for major != 3, got {data!r}" + _assert_server_still_usable() diff --git a/pgduck_server/tests/pytests/utils_protocol.py b/pgduck_server/tests/pytests/utils_protocol.py index aa03accc4..501b67730 100644 --- a/pgduck_server/tests/pytests/utils_protocol.py +++ b/pgduck_server/tests/pytests/utils_protocol.py @@ -1,5 +1,11 @@ import struct +# Wire protocol version numbers (major << 16 | minor). +PG_PROTOCOL_3_0 = (3 << 16) | 0 # 196608, the only version pgduck_server speaks +PG_PROTOCOL_3_2 = (3 << 16) | 2 # 196610, latest minor as of PG18/PG19 +PG_PROTOCOL_GREASE = (3 << 16) | 9999 # 206607, value PG18+ libpq probes with +PG_PROTOCOL_4_0 = (4 << 16) | 0 # 262144, an unsupported major version + def pack_message(message_type, payload): """Pack a message with the given type and payload for PostgreSQL protocol.""" @@ -12,12 +18,88 @@ def send_message(sock, message_type, payload=b""): sock.sendall(message) -def send_startup_message(sock): - """Send startup message to PostgreSQL.""" - payload = struct.pack("!I", 196608) # protocol version number +def send_startup_message(sock, protocol_version=PG_PROTOCOL_3_0, params=None): + """Send a startup message to PostgreSQL. + + With the defaults this reproduces the historical behaviour (a bare protocol + version with no parameters). Pass a higher protocol_version and/or a params + dict to exercise NegotiateProtocolVersion (PG18+ wire protocol 3.2 / GREASE + / _pq_.* protocol options). + """ + if protocol_version == PG_PROTOCOL_3_0 and params is None: + # Keep the exact bytes the other protocol tests have always sent. + send_message(sock, "", struct.pack("!I", protocol_version)) + return + + payload = struct.pack("!I", protocol_version) + for key, value in (params or {}).items(): + payload += key.encode() + b"\x00" + value.encode() + b"\x00" + payload += b"\x00" # final empty name terminates the parameter list send_message(sock, "", payload) +def recv_exact(sock, n): + """Read exactly n bytes, or fewer if the connection closes first.""" + buf = b"" + while len(buf) < n: + chunk = sock.recv(n - len(buf)) + if not chunk: + break + buf += chunk + return buf + + +def read_backend_message(sock): + """Read one backend message. + + Returns (type_char, payload_bytes), or (None, b"") if the connection closed + before a full message could be read. + """ + header = recv_exact(sock, 5) + if len(header) < 5: + return None, b"" + msg_type = chr(header[0]) + length = struct.unpack("!I", header[1:5])[0] + payload = recv_exact(sock, length - 4) + return msg_type, payload + + +def read_until(sock, target_type, max_messages=50): + """Read backend messages until one of `target_type` is seen. + + Returns the list of (type, payload) tuples read, including the target. + Raises AssertionError if the connection closes first or the target never + arrives within max_messages. + """ + seen = [] + for _ in range(max_messages): + msg_type, payload = read_backend_message(sock) + if msg_type is None: + raise AssertionError( + f"connection closed before {target_type!r}; saw {[m[0] for m in seen]}" + ) + seen.append((msg_type, payload)) + if msg_type == target_type: + return seen + raise AssertionError(f"did not see {target_type!r} within {max_messages} messages") + + +def parse_negotiate_protocol_version(payload): + """Parse a NegotiateProtocolVersion ('v') message payload. + + Returns (negotiated_version:int, [unrecognized_option_names]). + """ + version = struct.unpack("!I", payload[0:4])[0] + count = struct.unpack("!I", payload[4:8])[0] + names = [] + rest = payload[8:] + for _ in range(count): + nul = rest.index(b"\x00") + names.append(rest[:nul].decode()) + rest = rest[nul + 1 :] + return version, names + + def send_termination(sock): """Send termination message.""" send_message(sock, "X") From 486e72f64c0b1bfdb8d81fb81f8d9a3ea42e6c15 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:21:53 +0300 Subject: [PATCH 02/19] ci: match PG19 literally for the temporary pgaudit drop The pgaudit-on-PG19 workaround is temporary, so check for the literal major "19" instead of ">= 19". Co-authored-by: Cursor --- .github/actions/run-test/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/run-test/action.yml b/.github/actions/run-test/action.yml index 867f88911..db776650c 100644 --- a/.github/actions/run-test/action.yml +++ b/.github/actions/run-test/action.yml @@ -111,7 +111,7 @@ runs: PG_MAJOR="${{ inputs.pg_version }}" PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" PGAUDIT_ENABLED=true - if [[ "$PG_MAJOR" =~ ^[0-9]+$ ]] && [ "$PG_MAJOR" -ge 19 ]; then + if [ "$PG_MAJOR" = "19" ]; then PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pg_stat_statements" PGAUDIT_ENABLED=false fi From 70442ba767a4d8a92bd9b6f46d3198044a37262d Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:23:17 +0300 Subject: [PATCH 03/19] ci: build pgaudit for PG19 from the maintainer's PR branch pgaudit has no PG19 release yet, so build it from the maintainer's PG19 support branch (dwsteele/pgaudit#317, dev-pg19) instead of pinning main. That lets pgaudit stay preloaded on PG19 like every other version, so we can drop the PG19-only preload/enable workaround in the test action. We can switch to REL_19_STABLE once it is tagged. Co-authored-by: Cursor --- .github/actions/run-test/action.yml | 16 +--------------- docker/Dockerfile | 8 ++++++-- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/.github/actions/run-test/action.yml b/.github/actions/run-test/action.yml index db776650c..9052e1c4d 100644 --- a/.github/actions/run-test/action.yml +++ b/.github/actions/run-test/action.yml @@ -102,19 +102,7 @@ runs: if [ ${{ inputs.installcheck }} == 'true' ]; then pgduck_server --init_file_path pgduck_server/tests/test_secrets.sql --extensions_dir /tmp/extensions --cache_dir /tmp/cache > /tmp/pgduck_installcheck_logs 2>&1 & - # pgaudit has not yet added support for PostgreSQL 19's SQL/PGQ - # (graph_table); with pgaudit preloaded the backend crashes on the - # property-graph statements in PostgreSQL's own privileges regression - # test, cascading into the rest of the parallel group. Drop pgaudit - # from the preload list on PG19+ only (it stays on <=18). pgaudit only - # emits server-log entries, so this never changes regression output. - PG_MAJOR="${{ inputs.pg_version }}" PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" - PGAUDIT_ENABLED=true - if [ "$PG_MAJOR" = "19" ]; then - PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pg_stat_statements" - PGAUDIT_ENABLED=false - fi initdb -D /tmp/pg_installcheck_tests -U postgres --set "shared_preload_libraries=$PRELOAD_LIBS" --locale=C.UTF-8 --data-checksums @@ -125,9 +113,7 @@ runs: echo "pg_lake_iceberg.autovacuum_naptime=5" >> /tmp/pg_installcheck_tests/postgresql.conf echo "unix_socket_directories = '/tmp/pg_installcheck_tests'" >> /tmp/pg_installcheck_tests/postgresql.conf echo "max_prepared_transactions = 100" >> /tmp/pg_installcheck_tests/postgresql.conf - if [ "$PGAUDIT_ENABLED" == "true" ]; then - echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf - fi + echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf if [ ${{ inputs.installcheck_postgres }} == 'true' ]; then echo "compute_query_id='regress'" >> /tmp/pg_installcheck_tests/postgresql.conf diff --git a/docker/Dockerfile b/docker/Dockerfile index 6912cb26f..212bb9e97 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -17,7 +17,11 @@ ARG POSTGIS_VERSION=3.6.3 ARG PGAUDIT16_VERSION=REL_16_STABLE ARG PGAUDIT17_VERSION=REL_17_STABLE ARG PGAUDIT18_VERSION=REL_18_STABLE -ARG PGAUDIT19_VERSION=main +# PG19 support is not in a pgaudit release yet; build from the maintainer's +# PR branch (dwsteele/pgaudit#317, branch dev-pg19). Switch to REL_19_STABLE +# once it is tagged. +ARG PGAUDIT19_REPO=dwsteele/pgaudit +ARG PGAUDIT19_VERSION=dev-pg19 # Install build dependencies RUN if [ "$BASE_IMAGE_OS" = "almalinux" ]; then \ @@ -171,7 +175,7 @@ RUN wget https://ftp.postgresql.org/pub/source/v$PG16_VERSION/postgresql-$PG16_V wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT16_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT17_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT18_VERSION.tar.gz && \ - wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ + wget https://github.com/$PGAUDIT19_REPO/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ tar -xzf postgresql-$PG16_VERSION.tar.gz && rm postgresql-$PG16_VERSION.tar.gz && \ tar -xzf postgresql-$PG17_VERSION.tar.gz && rm postgresql-$PG17_VERSION.tar.gz && \ tar -xzf postgresql-$PG18_VERSION.tar.gz && rm postgresql-$PG18_VERSION.tar.gz && \ From c523bf0b82e046d707a5a26f03a6113f7e2d87bd Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:23:37 +0300 Subject: [PATCH 04/19] docker: build all pgaudit versions in one layer and drop the sources Combine the four per-version pgaudit build steps into a single RUN and remove the extracted source dirs afterwards to cut image layers and size. Co-authored-by: Cursor --- docker/Dockerfile | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 212bb9e97..0559583b8 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -244,21 +244,12 @@ RUN cd postgresql-$PG19_VERSION && \ (cd contrib && make -j `nproc` install) && \ cd .. && rm -rf postgresql-$PG19_VERSION* -# Compile and install PG AUDIT 16 -RUN cd pgaudit-$PGAUDIT16_VERSION && \ - make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config - -# Compile and install PG AUDIT 17 -RUN cd pgaudit-$PGAUDIT17_VERSION && \ - make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-17/bin/pg_config - -# Compile and install PG AUDIT 18 -RUN cd pgaudit-$PGAUDIT18_VERSION && \ - make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config - -# Compile and install PG AUDIT 19 -RUN cd pgaudit-$PGAUDIT19_VERSION && \ - make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config +# Compile and install pgaudit for every PostgreSQL version, then drop the sources +RUN (cd pgaudit-$PGAUDIT16_VERSION && make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-16/bin/pg_config) && \ + (cd pgaudit-$PGAUDIT17_VERSION && make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-17/bin/pg_config) && \ + (cd pgaudit-$PGAUDIT18_VERSION && make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-18/bin/pg_config) && \ + (cd pgaudit-$PGAUDIT19_VERSION && make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-19/bin/pg_config) && \ + rm -rf pgaudit-$PGAUDIT16_VERSION pgaudit-$PGAUDIT17_VERSION pgaudit-$PGAUDIT18_VERSION pgaudit-$PGAUDIT19_VERSION # Compile and install PostGIS RUN cd postgis-$POSTGIS_VERSION && \ From 56093e900010e42c484c1fac2a7f74a2867cf2e7 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:26:02 +0300 Subject: [PATCH 05/19] pg_extension_base: restore tranche init order Revert an accidental reordering of the tranche id / name assignments in BaseWorkerSharedMemoryInit so it matches main; the order is immaterial but the swap was unintentional. Co-authored-by: Cursor --- pg_extension_base/src/base_worker_launcher.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pg_extension_base/src/base_worker_launcher.c b/pg_extension_base/src/base_worker_launcher.c index 9e064a1d5..72ec21fa1 100644 --- a/pg_extension_base/src/base_worker_launcher.c +++ b/pg_extension_base/src/base_worker_launcher.c @@ -589,8 +589,9 @@ BaseWorkerSharedMemoryInit(void) if (!alreadyInitialized) { - BaseWorkerControl->lockTrancheName = "pg_extension_base server starter locks"; BaseWorkerControl->trancheId = LWLockNewTrancheId(); + BaseWorkerControl->lockTrancheName = "pg_extension_base server starter locks"; + LWLockRegisterTranche(BaseWorkerControl->trancheId, BaseWorkerControl->lockTrancheName); From 6227db937e30a0356a56ff2f64e7e7a6f00995e6 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:26:09 +0300 Subject: [PATCH 06/19] pg_extension_base: define PG_SIG_IGN shim for <= 18 PG19 added PG_SIG_IGN (SIG_IGN cast to pqsigfunc) for pqsignal(). Define the name as plain SIG_IGN on older releases in pg_compat.h so the SIGINT handler setup can use PG_SIG_IGN unconditionally instead of a per-call Co-authored-by: Cursor #if. --- pg_extension_base/include/pg_extension_base/pg_compat.h | 9 +++++++++ pg_extension_base/src/base_worker_launcher.c | 8 -------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pg_extension_base/include/pg_extension_base/pg_compat.h b/pg_extension_base/include/pg_extension_base/pg_compat.h index 562c3a107..3ed77f3b9 100644 --- a/pg_extension_base/include/pg_extension_base/pg_compat.h +++ b/pg_extension_base/include/pg_extension_base/pg_compat.h @@ -38,6 +38,15 @@ TupleDescFinalize(TupleDesc tupdesc) } #endif +/* + * PG19 added PG_SIG_IGN (a cast of SIG_IGN to pqsigfunc) for use with + * pqsignal(); older releases pass SIG_IGN directly. Define the name on + * <=18 so callers can use PG_SIG_IGN unconditionally. + */ +#if PG_VERSION_NUM < 190000 +#define PG_SIG_IGN SIG_IGN +#endif + #if PG_VERSION_NUM >= 190000 /* diff --git a/pg_extension_base/src/base_worker_launcher.c b/pg_extension_base/src/base_worker_launcher.c index 72ec21fa1..8548530be 100644 --- a/pg_extension_base/src/base_worker_launcher.c +++ b/pg_extension_base/src/base_worker_launcher.c @@ -732,11 +732,7 @@ PgExtensionServerStarterMain(Datum arg) /* set up signal handlers */ pqsignal(SIGHUP, HandleSighup); pqsignal(SIGTERM, HandleSigterm); -#if PG_VERSION_NUM >= 190000 pqsignal(SIGINT, PG_SIG_IGN); -#else - pqsignal(SIGINT, SIG_IGN); -#endif before_shmem_exit(PgBaseExtensionServerStarterSharedMemoryExit, 0); @@ -1098,11 +1094,7 @@ PgExtensionBaseDatabaseStarterMain(Datum databaseIdDatum) /* Establish signal handlers before unblocking signals. */ pqsignal(SIGHUP, HandleSighup); -#if PG_VERSION_NUM >= 190000 pqsignal(SIGINT, PG_SIG_IGN); -#else - pqsignal(SIGINT, SIG_IGN); -#endif pqsignal(SIGTERM, HandleSigterm); /* Set our exit handler before any calls to proc_exit */ From ef0725224dd7abfd726fac86786edf941b3d3359 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:26:16 +0300 Subject: [PATCH 07/19] pg_lake_engine: format pg_plan_query call one arg per line Spread the pg_plan_query() arguments onto their own lines so the PG19-only trailing NULL argument reads cleanly instead of being appended inline. Co-authored-by: Cursor --- pg_lake_engine/src/query/execute.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pg_lake_engine/src/query/execute.c b/pg_lake_engine/src/query/execute.c index 3bd3730ee..6de1635a1 100644 --- a/pg_lake_engine/src/query/execute.c +++ b/pg_lake_engine/src/query/execute.c @@ -75,7 +75,10 @@ uint64 ExecuteQueryToDestReceiver(Query *query, const char *queryString, ParamListInfo params, DestReceiver *dest) { - PlannedStmt *plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, params + PlannedStmt *plan = pg_plan_query(query, + queryString, + CURSOR_OPT_PARALLEL_OK, + params #if PG_VERSION_NUM >= 190000 ,NULL #endif From 7f47dce1eb91074fe83579669d3f69f12b4c35f7 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:28:45 +0300 Subject: [PATCH 08/19] tests: drop the PG19 version constant Inline the 190000 server-version literal in the PG19 skip helpers instead of defining a one-off PG19 module constant, matching how the rest of the suite spells version checks. Co-authored-by: Cursor --- pg_lake_copy/tests/pytests/test_parquet_copy.py | 5 +---- pg_lake_copy/tests/pytests/test_pg19_copy_options.py | 4 +--- pg_lake_table/tests/pytests/test_pg19_new_features.py | 5 +---- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/pg_lake_copy/tests/pytests/test_parquet_copy.py b/pg_lake_copy/tests/pytests/test_parquet_copy.py index 958aab769..992f4699e 100644 --- a/pg_lake_copy/tests/pytests/test_parquet_copy.py +++ b/pg_lake_copy/tests/pytests/test_parquet_copy.py @@ -5,11 +5,8 @@ import math from utils_pytest import * -PG19 = 190000 - - def _skip_pre_pg19(conn): - if get_pg_version_num(conn) < PG19: + if get_pg_version_num(conn) < 190000: pytest.skip("COPY TO requires PG19+") diff --git a/pg_lake_copy/tests/pytests/test_pg19_copy_options.py b/pg_lake_copy/tests/pytests/test_pg19_copy_options.py index bc42b7dc4..beeae12d4 100644 --- a/pg_lake_copy/tests/pytests/test_pg19_copy_options.py +++ b/pg_lake_copy/tests/pytests/test_pg19_copy_options.py @@ -11,11 +11,9 @@ import pytest from utils_pytest import * -PG19 = 190000 - def _skip_pre_pg19(conn): - if get_pg_version_num(conn) < PG19: + if get_pg_version_num(conn) < 190000: pytest.skip("PG19 COPY option behavior") diff --git a/pg_lake_table/tests/pytests/test_pg19_new_features.py b/pg_lake_table/tests/pytests/test_pg19_new_features.py index 2a14b7a2b..bd60d8b80 100644 --- a/pg_lake_table/tests/pytests/test_pg19_new_features.py +++ b/pg_lake_table/tests/pytests/test_pg19_new_features.py @@ -13,11 +13,8 @@ import pytest from utils_pytest import * -PG19 = 190000 - - def _skip_if_not_pg19(conn): - if get_pg_version_num(conn) < PG19: + if get_pg_version_num(conn) < 190000: pytest.skip("PG19-only commands require PostgreSQL 19+") From 87ffea3dbe427fde6b078f211219620f4545c09a Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:29:22 +0300 Subject: [PATCH 09/19] tests: drop pg19 branding from partitioned COPY test names These COPY-to-partitioned-table tests will outlive PG19, and the version guard already skips them where unsupported, so rename the functions and helper to describe the behavior instead of the release. Co-authored-by: Cursor --- .../tests/pytests/test_parquet_copy.py | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/pg_lake_copy/tests/pytests/test_parquet_copy.py b/pg_lake_copy/tests/pytests/test_parquet_copy.py index 992f4699e..6d62ec57a 100644 --- a/pg_lake_copy/tests/pytests/test_parquet_copy.py +++ b/pg_lake_copy/tests/pytests/test_parquet_copy.py @@ -5,7 +5,8 @@ import math from utils_pytest import * -def _skip_pre_pg19(conn): + +def _skip_if_partitioned_copy_unsupported(conn): if get_pg_version_num(conn) < 190000: pytest.skip("COPY TO requires PG19+") @@ -747,10 +748,10 @@ def test_partitioned(pg_conn, duckdb_conn, tmp_path): pg_conn.rollback() -def test_pg19_copy_to_all_heap_partitioned(pg_conn, tmp_path): +def test_copy_to_all_heap_partitioned(pg_conn, tmp_path): """PG19: COPY TO with all-heap partitions streams every partition's rows out, and round-trips into a plain table.""" - _skip_pre_pg19(pg_conn) + _skip_if_partitioned_copy_unsupported(pg_conn) out_path = tmp_path / "all_heap.parquet" run_command( @@ -787,15 +788,15 @@ def test_pg19_copy_to_all_heap_partitioned(pg_conn, tmp_path): pg_conn.rollback() -def test_pg19_copy_to_mixed_partition_tree(pg_conn, s3, extension, tmp_path): +def test_copy_to_mixed_partition_tree(pg_conn, s3, extension, tmp_path): """PG19 HEADLINE: COPY TO where leaves are a mix of a heap partition and a pg_lake foreign-table partition (parquet on s3). The inh=true descent must read the foreign child through the FDW so ALL rows appear in the output.""" - _skip_pre_pg19(pg_conn) + _skip_if_partitioned_copy_unsupported(pg_conn) - ft_url = f"s3://{TEST_BUCKET}/test_pg19_copy_mixed/ft_child.parquet" - out_url = f"s3://{TEST_BUCKET}/test_pg19_copy_mixed/parent_out.parquet" + ft_url = f"s3://{TEST_BUCKET}/test_copy_mixed/ft_child.parquet" + out_url = f"s3://{TEST_BUCKET}/test_copy_mixed/parent_out.parquet" # Write the parquet backing the foreign-table partition (ids 100..199). run_command( @@ -854,15 +855,15 @@ def test_pg19_copy_to_mixed_partition_tree(pg_conn, s3, extension, tmp_path): pg_conn.commit() -def test_pg19_copy_to_mixed_tree_with_iceberg_child( +def test_copy_to_mixed_tree_with_iceberg_child( pg_conn, s3, with_default_location, tmp_path ): """PG19: like the mixed-tree test but one leaf is an iceberg child created via PARTITION OF ... USING iceberg. Proves the descent reads iceberg partitions through the FDW too.""" - _skip_pre_pg19(pg_conn) + _skip_if_partitioned_copy_unsupported(pg_conn) - out_url = f"s3://{TEST_BUCKET}/test_pg19_copy_iceberg/parent_out.parquet" + out_url = f"s3://{TEST_BUCKET}/test_copy_iceberg/parent_out.parquet" run_command( """ @@ -912,10 +913,10 @@ def test_pg19_copy_to_mixed_tree_with_iceberg_child( pg_conn.commit() -def test_pg19_copy_to_column_subset(pg_conn, tmp_path): +def test_copy_to_column_subset(pg_conn, tmp_path): """PG19: COPY parent (col_a, col_b) TO ... must descend into partitions and project only the requested columns.""" - _skip_pre_pg19(pg_conn) + _skip_if_partitioned_copy_unsupported(pg_conn) out_path = tmp_path / "subset.parquet" run_command( @@ -953,11 +954,11 @@ def test_pg19_copy_to_column_subset(pg_conn, tmp_path): pg_conn.rollback() -def test_pg19_copy_only_parent_yields_zero_rows(pg_conn, tmp_path): +def test_copy_only_parent_yields_zero_rows(pg_conn, tmp_path): """PG19: COPY ONLY TO ... honors ONLY semantics -- the parent itself holds no rows, so the output is empty even though the partitions are populated.""" - _skip_pre_pg19(pg_conn) + _skip_if_partitioned_copy_unsupported(pg_conn) out_path = tmp_path / "only_parent.parquet" run_command( From 998a9d8f1a3e663c74f3e898af01d790c50d6143 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:32:23 +0300 Subject: [PATCH 10/19] tests: verify REPACK leaves pg_lake table contents intact Compare the full, deterministically-ordered table contents before and after REPACK instead of only the row count, so a silent rewrite that preserved cardinality but corrupted rows would be caught. Co-authored-by: Cursor --- .../tests/pytests/test_pg19_new_features.py | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/pg_lake_table/tests/pytests/test_pg19_new_features.py b/pg_lake_table/tests/pytests/test_pg19_new_features.py index bd60d8b80..2b599d639 100644 --- a/pg_lake_table/tests/pytests/test_pg19_new_features.py +++ b/pg_lake_table/tests/pytests/test_pg19_new_features.py @@ -13,6 +13,7 @@ import pytest from utils_pytest import * + def _skip_if_not_pg19(conn): if get_pg_version_num(conn) < 190000: pytest.skip("PG19-only commands require PostgreSQL 19+") @@ -35,14 +36,26 @@ def _make_parquet_foreign_table(conn, name, select_sql, columns_sql): return url +def _snapshot_contents(conn, relation): + """Return the full table contents as a deterministically-ordered list of + row literals, so two snapshots can be compared for exact equality.""" + rows = run_query(f"SELECT t::text AS row FROM {relation} t ORDER BY 1", conn) + conn.commit() + return [r["row"] for r in rows] + + def _assert_rejected_or_harmless(conn, relation, repack_sql, expected_count): """REPACK must either raise a meaningful error or leave the data intact. pg_lake tables are foreign tables under the hood, so PostgreSQL core is - expected to reject REPACK on them. We accept a clean error *or* a no-op - that preserves the row count; we reject silent data loss/corruption and - crashes. + expected to reject REPACK on them. We accept a clean error *or* a no-op, + but in either case the full table contents (not just the row count) must be + byte-for-byte unchanged afterwards; we reject silent data loss/corruption + and crashes. """ + before = _snapshot_contents(conn, relation) + assert len(before) == expected_count + error = run_command(repack_sql, conn, raise_error=False) conn.rollback() @@ -59,18 +72,13 @@ def _assert_rejected_or_harmless(conn, relation, repack_sql, expected_count): "is not", ) ), f"REPACK on {relation} raised an unclear error: {error!r}" - else: - # If it "succeeded", the table must still be readable and intact. - rows = run_query(f"SELECT count(*) AS n FROM {relation}", conn) - conn.commit() - assert ( - int(rows[0]["n"]) == expected_count - ), f"REPACK on {relation} changed the row count unexpectedly" - - # The server must still be alive and the table still queryable. - rows = run_query(f"SELECT count(*) AS n FROM {relation}", conn) - conn.commit() - assert int(rows[0]["n"]) == expected_count + + # Whether REPACK errored or silently no-op'd, the server must still be + # alive and the table contents identical to the pre-REPACK snapshot. + after = _snapshot_contents(conn, relation) + assert ( + after == before + ), f"REPACK on {relation} changed the table contents unexpectedly" def test_repack_managed_iceberg_table(pg_conn, s3, extension, with_default_location): From 6042462caa42eabc307759e343fd659ef722375d Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 17:32:23 +0300 Subject: [PATCH 11/19] tests: clarify protocol negotiation docstring Wire protocol 3.2 arrived in PG18 and the GREASE/_pq_ probing that forces pgduck_server to answer NegotiateProtocolVersion is new in PG19; the 'v' message itself predates both. Reword so the versions are attributed correctly. Co-authored-by: Cursor --- .../tests/pytests/test_protocol_negotiation.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pgduck_server/tests/pytests/test_protocol_negotiation.py b/pgduck_server/tests/pytests/test_protocol_negotiation.py index ffedb8dba..b94562f53 100644 --- a/pgduck_server/tests/pytests/test_protocol_negotiation.py +++ b/pgduck_server/tests/pytests/test_protocol_negotiation.py @@ -1,10 +1,12 @@ """Wire-protocol coverage for pgduck_server's NegotiateProtocolVersion handling. -PostgreSQL 18 introduced wire protocol 3.2 together with the -NegotiateProtocolVersion ('v') backend message, and PG18+ libpq probes servers -by requesting a higher minor version than it strictly needs -(PG_PROTOCOL_GREASE = 3.9999) and/or by sending forward-compatible "_pq_.*" -protocol options. A server that only speaks 3.0 -- like pgduck_server -- must: +The NegotiateProtocolVersion ('v') backend message itself predates these +releases, but two newer changes made pgduck_server actually have to handle it. +PostgreSQL 18 bumped the latest wire protocol to 3.2, and PostgreSQL 19's libpq +now probes servers on connect by requesting a deliberately-too-high minor +version (PG_PROTOCOL_GREASE = 3.9999) and/or by sending forward-compatible +"_pq_.*" protocol options. A server that only speaks 3.0 -- like pgduck_server +-- must: * accept the connection, * reply with a 'v' message advertising the version it actually speaks (3.0), From 611bf7db68e29fa5e3476df1bba562a1b80c700d Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Fri, 19 Jun 2026 18:33:11 +0300 Subject: [PATCH 12/19] ci: keep pgaudit out of the PG19 preload The dev-pg19 pgaudit branch builds, but CI showed it still segfaults the backend while auditing PG19's SQL/PGQ graph_table statements in PostgreSQL's own privileges regression test (109/245 upstream tests cascade-failed). Restore the PG19-only pgaudit drop (using a literal "19" check) and build plain pgaudit main in the image until upstream actually supports PG19. Co-authored-by: Cursor --- .github/actions/run-test/action.yml | 18 +++++++++++++++++- docker/Dockerfile | 12 ++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.github/actions/run-test/action.yml b/.github/actions/run-test/action.yml index 9052e1c4d..5fb615f0b 100644 --- a/.github/actions/run-test/action.yml +++ b/.github/actions/run-test/action.yml @@ -102,7 +102,21 @@ runs: if [ ${{ inputs.installcheck }} == 'true' ]; then pgduck_server --init_file_path pgduck_server/tests/test_secrets.sql --extensions_dir /tmp/extensions --cache_dir /tmp/cache > /tmp/pgduck_installcheck_logs 2>&1 & + # pgaudit has no PostgreSQL 19 support yet -- not in a release, and not + # even in the maintainer's in-progress PG19 branch (dwsteele/pgaudit#317, + # dev-pg19), which compiles but still segfaults the backend while + # auditing PG19's SQL/PGQ graph_table statements in PostgreSQL's own + # privileges regression test, cascading through the parallel group. + # Drop pgaudit from the preload list on PG19 only (it stays on <=18). + # pgaudit only emits server-log entries, so this never changes + # regression output. + PG_MAJOR="${{ inputs.pg_version }}" PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" + PGAUDIT_ENABLED=true + if [ "$PG_MAJOR" = "19" ]; then + PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pg_stat_statements" + PGAUDIT_ENABLED=false + fi initdb -D /tmp/pg_installcheck_tests -U postgres --set "shared_preload_libraries=$PRELOAD_LIBS" --locale=C.UTF-8 --data-checksums @@ -113,7 +127,9 @@ runs: echo "pg_lake_iceberg.autovacuum_naptime=5" >> /tmp/pg_installcheck_tests/postgresql.conf echo "unix_socket_directories = '/tmp/pg_installcheck_tests'" >> /tmp/pg_installcheck_tests/postgresql.conf echo "max_prepared_transactions = 100" >> /tmp/pg_installcheck_tests/postgresql.conf - echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf + if [ "$PGAUDIT_ENABLED" == "true" ]; then + echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf + fi if [ ${{ inputs.installcheck_postgres }} == 'true' ]; then echo "compute_query_id='regress'" >> /tmp/pg_installcheck_tests/postgresql.conf diff --git a/docker/Dockerfile b/docker/Dockerfile index 0559583b8..d113fa3a5 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -17,11 +17,11 @@ ARG POSTGIS_VERSION=3.6.3 ARG PGAUDIT16_VERSION=REL_16_STABLE ARG PGAUDIT17_VERSION=REL_17_STABLE ARG PGAUDIT18_VERSION=REL_18_STABLE -# PG19 support is not in a pgaudit release yet; build from the maintainer's -# PR branch (dwsteele/pgaudit#317, branch dev-pg19). Switch to REL_19_STABLE -# once it is tagged. -ARG PGAUDIT19_REPO=dwsteele/pgaudit -ARG PGAUDIT19_VERSION=dev-pg19 +# pgaudit has no PG19 release yet. We build main here, but it is intentionally +# kept out of the PG19 preload list at test time (see run-test/action.yml): +# even the maintainer's dev-pg19 branch still crashes the backend on PG19's +# SQL/PGQ graph_table audit path. +ARG PGAUDIT19_VERSION=main # Install build dependencies RUN if [ "$BASE_IMAGE_OS" = "almalinux" ]; then \ @@ -175,7 +175,7 @@ RUN wget https://ftp.postgresql.org/pub/source/v$PG16_VERSION/postgresql-$PG16_V wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT16_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT17_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT18_VERSION.tar.gz && \ - wget https://github.com/$PGAUDIT19_REPO/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ + wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ tar -xzf postgresql-$PG16_VERSION.tar.gz && rm postgresql-$PG16_VERSION.tar.gz && \ tar -xzf postgresql-$PG17_VERSION.tar.gz && rm postgresql-$PG17_VERSION.tar.gz && \ tar -xzf postgresql-$PG18_VERSION.tar.gz && rm postgresql-$PG18_VERSION.tar.gz && \ From 0fa7de0c425be722e4f094086afc9b382e5770b8 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Mon, 22 Jun 2026 11:59:25 +0300 Subject: [PATCH 13/19] rebase: restore includes lost in the rest_catalog.c split main #406 split rest_catalog.c into sibling files. The PG19 implicit- include hardening needs access/htup_details.h (GETSTRUCT) in the DDL sibling and pg_extension_base/pg_compat.h (numeric_int4_opt_error shim) in the HTTP sibling, which the old monolithic file pulled in transitively. Co-authored-by: Cursor --- pg_lake_iceberg/src/rest_catalog/rest_catalog_ddl.c | 1 + pg_lake_iceberg/src/rest_catalog/rest_catalog_http.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/pg_lake_iceberg/src/rest_catalog/rest_catalog_ddl.c b/pg_lake_iceberg/src/rest_catalog/rest_catalog_ddl.c index 5dfe5a405..ae4db3225 100644 --- a/pg_lake_iceberg/src/rest_catalog/rest_catalog_ddl.c +++ b/pg_lake_iceberg/src/rest_catalog/rest_catalog_ddl.c @@ -25,6 +25,7 @@ #include "postgres.h" #include "access/genam.h" +#include "access/htup_details.h" #include "access/table.h" #include "catalog/pg_class.h" #include "catalog/pg_depend.h" diff --git a/pg_lake_iceberg/src/rest_catalog/rest_catalog_http.c b/pg_lake_iceberg/src/rest_catalog/rest_catalog_http.c index 20176641d..ee27f9a60 100644 --- a/pg_lake_iceberg/src/rest_catalog/rest_catalog_http.c +++ b/pg_lake_iceberg/src/rest_catalog/rest_catalog_http.c @@ -35,6 +35,8 @@ #include "postgres.h" +#include "pg_extension_base/pg_compat.h" + #include "fmgr.h" #include "lib/stringinfo.h" #include "utils/builtins.h" From bf7e3e62e1cbbf26ee1d85752c6f34f06d6de843 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Mon, 22 Jun 2026 11:59:39 +0300 Subject: [PATCH 14/19] pg_lake_engine: use version-specific pg_plan_query calls Replace the inline "#if ... ,NULL" spliced argument with two complete pg_plan_query() calls, one per major, per review feedback. Co-authored-by: Cursor --- pg_lake_engine/src/query/execute.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pg_lake_engine/src/query/execute.c b/pg_lake_engine/src/query/execute.c index 6de1635a1..8558ce0b1 100644 --- a/pg_lake_engine/src/query/execute.c +++ b/pg_lake_engine/src/query/execute.c @@ -75,14 +75,13 @@ uint64 ExecuteQueryToDestReceiver(Query *query, const char *queryString, ParamListInfo params, DestReceiver *dest) { - PlannedStmt *plan = pg_plan_query(query, - queryString, - CURSOR_OPT_PARALLEL_OK, - params #if PG_VERSION_NUM >= 190000 - ,NULL + PlannedStmt *plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, + params, NULL); +#else + PlannedStmt *plan = pg_plan_query(query, queryString, CURSOR_OPT_PARALLEL_OK, + params); #endif - ); return ExecutePlanToDestReceiver(plan, queryString, params, dest); } From 65fb166b8de801f5ff4c94eb1df0abfcd2d39be4 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Mon, 22 Jun 2026 11:59:39 +0300 Subject: [PATCH 15/19] pg_lake_copy: define PostgresSupportsJsonCopy as false on < PG19 Give the helper a constant-false definition for pre-19 so the JSON routing in IsPgLakeCopy no longer needs its own version guard; the only PG19 #if now lives at the declaration/definition. Co-authored-by: Cursor --- pg_lake_copy/src/copy/copy.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/pg_lake_copy/src/copy/copy.c b/pg_lake_copy/src/copy/copy.c index 44881e393..d930ba188 100644 --- a/pg_lake_copy/src/copy/copy.c +++ b/pg_lake_copy/src/copy/copy.c @@ -101,6 +101,10 @@ PgLakeCopyValidityCheckHookType PgLakeCopyValidityCheckHook = NULL; static bool IsPgLakeCopy(CopyStmt *copyStmt); #if PG_VERSION_NUM >= 190000 static bool PostgresSupportsJsonCopy(CopyStmt *copyStmt); +#else + +/* pre-19 Postgres has no native JSON COPY, so it never claims the case */ +#define PostgresSupportsJsonCopy(copyStmt) false #endif static bool IsCopyFromStdin(CopyStmt *copyStmt); static bool IsCopyToStdout(CopyStmt *copyStmt); @@ -267,15 +271,14 @@ IsPgLakeCopy(CopyStmt *copyStmt) return false; } -#if PG_VERSION_NUM >= 190000 - /* - * On PG19+ Postgres has a native JSON COPY for STDIN/STDOUT/local files - * (a supported URL/@stage already returned true above, so by here the - * target is local). Mirror the CSV rule: give Postgres precedence - * whenever it supports the case, and let pg_lake cover the rest. The - * hidden, test-only json_copy_mode GUC overrides this for the regression - * and pg_lake test suites. + * PG19+ has a native JSON COPY for STDIN/STDOUT/local files (a supported + * URL/@stage already returned true above, so by here the target is + * local). Mirror the CSV rule: give Postgres precedence whenever it + * supports the case, and let pg_lake cover the rest. On <=18 + * PostgresSupportsJsonCopy() is a constant false, so pg_lake always + * handles JSON there. The hidden, test-only json_copy_mode GUC overrides + * this for the regression and pg_lake test suites. */ if (format == DATA_FORMAT_JSON) { @@ -299,7 +302,6 @@ IsPgLakeCopy(CopyStmt *copyStmt) return true; } } -#endif return true; } From 88c51544ae929a81d8ecbb7000f3600ff7155e8a Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Mon, 22 Jun 2026 11:59:39 +0300 Subject: [PATCH 16/19] pg_extension_base: include utils/wait_event.h unconditionally The include just covers PG19's stricter implicit-include rules for WAIT_EVENT_CLIENT_READ; utils/wait_event.h has existed since PG14, so drop the version guard. Co-authored-by: Cursor --- pg_extension_base/src/base_worker_launcher.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/pg_extension_base/src/base_worker_launcher.c b/pg_extension_base/src/base_worker_launcher.c index 8548530be..08981feb9 100644 --- a/pg_extension_base/src/base_worker_launcher.c +++ b/pg_extension_base/src/base_worker_launcher.c @@ -98,9 +98,7 @@ #include "utils/snapmgr.h" #include "utils/syscache.h" #include "utils/tuplestore.h" -#if PG_VERSION_NUM >= 190000 #include "utils/wait_event.h" -#endif #include "tcop/utility.h" #include "pg_extension_base/base_workers.h" From ec1af9fa263833c51a48346516f711d9af8c65ae Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Mon, 22 Jun 2026 11:59:39 +0300 Subject: [PATCH 17/19] pg_lake_table: include utils/hsearch.h for HTAB in the catalog header The header only needs the HTAB typedef for its signatures, not the dynahash internals. Include utils/hsearch.h unconditionally and let callers pull in utils/dynahash.h where they actually use it. Co-authored-by: Cursor --- pg_lake_table/include/pg_lake/fdw/data_files_catalog.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h b/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h index 5db9e2d4a..1a46c94e0 100644 --- a/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h +++ b/pg_lake_table/include/pg_lake/fdw/data_files_catalog.h @@ -22,9 +22,7 @@ #include "pg_lake/util/s3_reader_utils.h" #include "nodes/pg_list.h" -#if PG_VERSION_NUM < 190000 -#include "utils/dynahash.h" -#endif +#include "utils/hsearch.h" #define DATA_FILES_TABLE_QUALIFIED \ PG_LAKE_TABLE_SCHEMA "." PG_LAKE_TABLE_FILES_TABLE_NAME From ab0a5343e09ad501ab41597872c035a668ac3d75 Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Mon, 22 Jun 2026 11:59:39 +0300 Subject: [PATCH 18/19] pg_lake_table: drop empty else branch in iceberg column mapping palloc0 already zeroes the mapping, so the no-matching-column case needs no body. Fold the explanation into the guard and remove the empty else. Co-authored-by: Cursor --- .../fdw/schema_operations/register_field_ids.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pg_lake_table/src/fdw/schema_operations/register_field_ids.c b/pg_lake_table/src/fdw/schema_operations/register_field_ids.c index 2fe80c2f7..036dca195 100644 --- a/pg_lake_table/src/fdw/schema_operations/register_field_ids.c +++ b/pg_lake_table/src/fdw/schema_operations/register_field_ids.c @@ -276,6 +276,15 @@ CreatePostgresColumnMappingsForIcebergTableFromExternalMetadata(Oid relationId) columnMapping->attname = pstrdup(field->name); columnMapping->attrNum = get_attnum(relationId, field->name); + /* + * A managed (non-REST) Iceberg table can carry a field that has no + * matching Postgres column (the REST read-only case already skipped + * those above). Only look up the attribute when it exists: the + * type/null/default fields stay zero (from palloc0) so + * ErrorIfSchemasDoNotMatch() reports the mismatch, and we avoid + * TupleDescAttr(InvalidAttrNumber - 1), which trips PG19's new bounds + * Assert in TupleDescAttr(). + */ if (columnMapping->attrNum != InvalidAttrNumber) { Form_pg_attribute attr = TupleDescAttr(tupDesc, columnMapping->attrNum - 1); @@ -284,15 +293,6 @@ CreatePostgresColumnMappingsForIcebergTableFromExternalMetadata(Oid relationId) columnMapping->attNotNull = attr->attnotnull; columnMapping->attHasDef = attr->atthasdef; } - else - { - /* - * No matching Postgres column. Leave the type/null/default fields - * zero so ErrorIfSchemasDoNotMatch() reports the mismatch, and - * avoid TupleDescAttr(InvalidAttrNumber - 1), which trips PG19's - * new bounds Assert in TupleDescAttr(). - */ - } pgColumnMappingList = lappend(pgColumnMappingList, columnMapping); } From 4dd6afe58a15027d1d59360c58aaff256b20254a Mon Sep 17 00:00:00 2001 From: Onder KALACI Date: Tue, 23 Jun 2026 09:47:10 +0300 Subject: [PATCH 19/19] ci: re-enable pgaudit on PG19 The maintainer fixed the SQL/PGQ graph_table audit crash on the dev-pg19 branch (dwsteele/pgaudit#317, commit ffbae89), so we no longer need to keep pgaudit out of the PG19 preload. Build pgaudit for PG19 from that branch and preload it on PG19 like every other version. Verified locally on PG19beta1: a GRAPH_TABLE query over a property graph is now audited instead of aborting the backend. Co-authored-by: Cursor --- .github/actions/run-test/action.yml | 18 +----------------- docker/Dockerfile | 13 +++++++------ 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/.github/actions/run-test/action.yml b/.github/actions/run-test/action.yml index 5fb615f0b..9052e1c4d 100644 --- a/.github/actions/run-test/action.yml +++ b/.github/actions/run-test/action.yml @@ -102,21 +102,7 @@ runs: if [ ${{ inputs.installcheck }} == 'true' ]; then pgduck_server --init_file_path pgduck_server/tests/test_secrets.sql --extensions_dir /tmp/extensions --cache_dir /tmp/cache > /tmp/pgduck_installcheck_logs 2>&1 & - # pgaudit has no PostgreSQL 19 support yet -- not in a release, and not - # even in the maintainer's in-progress PG19 branch (dwsteele/pgaudit#317, - # dev-pg19), which compiles but still segfaults the backend while - # auditing PG19's SQL/PGQ graph_table statements in PostgreSQL's own - # privileges regression test, cascading through the parallel group. - # Drop pgaudit from the preload list on PG19 only (it stays on <=18). - # pgaudit only emits server-log entries, so this never changes - # regression output. - PG_MAJOR="${{ inputs.pg_version }}" PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" - PGAUDIT_ENABLED=true - if [ "$PG_MAJOR" = "19" ]; then - PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pg_stat_statements" - PGAUDIT_ENABLED=false - fi initdb -D /tmp/pg_installcheck_tests -U postgres --set "shared_preload_libraries=$PRELOAD_LIBS" --locale=C.UTF-8 --data-checksums @@ -127,9 +113,7 @@ runs: echo "pg_lake_iceberg.autovacuum_naptime=5" >> /tmp/pg_installcheck_tests/postgresql.conf echo "unix_socket_directories = '/tmp/pg_installcheck_tests'" >> /tmp/pg_installcheck_tests/postgresql.conf echo "max_prepared_transactions = 100" >> /tmp/pg_installcheck_tests/postgresql.conf - if [ "$PGAUDIT_ENABLED" == "true" ]; then - echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf - fi + echo "pgaudit.log='all'" >> /tmp/pg_installcheck_tests/postgresql.conf if [ ${{ inputs.installcheck_postgres }} == 'true' ]; then echo "compute_query_id='regress'" >> /tmp/pg_installcheck_tests/postgresql.conf diff --git a/docker/Dockerfile b/docker/Dockerfile index d113fa3a5..86156a3da 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -17,11 +17,12 @@ ARG POSTGIS_VERSION=3.6.3 ARG PGAUDIT16_VERSION=REL_16_STABLE ARG PGAUDIT17_VERSION=REL_17_STABLE ARG PGAUDIT18_VERSION=REL_18_STABLE -# pgaudit has no PG19 release yet. We build main here, but it is intentionally -# kept out of the PG19 preload list at test time (see run-test/action.yml): -# even the maintainer's dev-pg19 branch still crashes the backend on PG19's -# SQL/PGQ graph_table audit path. -ARG PGAUDIT19_VERSION=main +# pgaudit has no PG19 release yet, so build the maintainer's PG19 support branch +# (dwsteele/pgaudit#317, dev-pg19). It now fixes the SQL/PGQ graph_table audit +# crash (commit ffbae89), so pgaudit can stay preloaded on PG19 like the other +# versions. Switch to REL_19_STABLE once it is tagged. +ARG PGAUDIT19_REPO=dwsteele/pgaudit +ARG PGAUDIT19_VERSION=dev-pg19 # Install build dependencies RUN if [ "$BASE_IMAGE_OS" = "almalinux" ]; then \ @@ -175,7 +176,7 @@ RUN wget https://ftp.postgresql.org/pub/source/v$PG16_VERSION/postgresql-$PG16_V wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT16_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT17_VERSION.tar.gz && \ wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT18_VERSION.tar.gz && \ - wget https://github.com/pgaudit/pgaudit/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ + wget https://github.com/$PGAUDIT19_REPO/archive/refs/heads/$PGAUDIT19_VERSION.tar.gz && \ tar -xzf postgresql-$PG16_VERSION.tar.gz && rm postgresql-$PG16_VERSION.tar.gz && \ tar -xzf postgresql-$PG17_VERSION.tar.gz && rm postgresql-$PG17_VERSION.tar.gz && \ tar -xzf postgresql-$PG18_VERSION.tar.gz && rm postgresql-$PG18_VERSION.tar.gz && \