diff --git a/.github/actions/run-test/action.yml b/.github/actions/run-test/action.yml index a148b4325..9052e1c4d 100644 --- a/.github/actions/run-test/action.yml +++ b/.github/actions/run-test/action.yml @@ -102,7 +102,9 @@ 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 + PRELOAD_LIBS="pg_extension_base,pg_cron,pg_lake_table,pg_lake_copy,pg_lake,pgaudit,pg_stat_statements" + + 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 @@ -116,6 +118,21 @@ runs: 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..86156a3da 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -9,13 +9,20 @@ 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 +# 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 \ @@ -28,6 +35,7 @@ RUN if [ "$BASE_IMAGE_OS" = "almalinux" ]; then \ ninja-build \ wget \ git \ + bzip2 \ readline-devel \ zlib-devel \ flex \ @@ -73,6 +81,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 +137,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 +167,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/$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 && \ + 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,31 +232,44 @@ RUN cd postgresql-$PG18_VERSION && \ (cd contrib && make -j `nproc` install) && \ cd .. && rm -rf postgresql-$PG18_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 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 18 -RUN cd pgaudit-$PGAUDIT18_VERSION && \ - make -j `nproc` install USE_PGXS=1 PG_CONFIG=$PGBASEDIR/pgsql-18/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 && \ ./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 +278,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..3ed77f3b9 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,83 @@ #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 + +/* + * 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 + +/* + * 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..08981feb9 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,8 @@ #include "utils/memutils.h" #include "utils/snapmgr.h" #include "utils/syscache.h" +#include "utils/tuplestore.h" +#include "utils/wait_event.h" #include "tcop/utility.h" #include "pg_extension_base/base_workers.h" @@ -292,8 +297,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 +388,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 +491,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 +510,8 @@ InitializeBaseWorkerLauncher(void) } +#if PG_VERSION_NUM < 190000 + /* * BaseWorkerSharedMemorySize computes how much shared memory is required. */ @@ -493,6 +568,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) @@ -542,6 +621,7 @@ BaseWorkerSharedMemoryInit(void) LWLockRelease(AddinShmemInitLock); } +#endif /* @@ -650,7 +730,7 @@ PgExtensionServerStarterMain(Datum arg) /* set up signal handlers */ pqsignal(SIGHUP, HandleSighup); pqsignal(SIGTERM, HandleSigterm); - pqsignal(SIGINT, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); before_shmem_exit(PgBaseExtensionServerStarterSharedMemoryExit, 0); @@ -1012,7 +1092,7 @@ PgExtensionBaseDatabaseStarterMain(Datum databaseIdDatum) /* Establish signal handlers before unblocking signals. */ pqsignal(SIGHUP, HandleSighup); - pqsignal(SIGINT, SIG_IGN); + pqsignal(SIGINT, PG_SIG_IGN); 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..d930ba188 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,13 @@ 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); static void ProcessPgLakeCopyFrom(CopyStmt *copyStmt, ParseState *pstate, @@ -139,6 +149,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 +258,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 +271,89 @@ IsPgLakeCopy(CopyStmt *copyStmt) return false; } + /* + * 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) + { + 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; + } + } + 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 +1141,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 +1289,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 +1410,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 +1496,8 @@ BuildTupleDescriptorForRelation(Relation relation, List *attributeList) attributeNumber++; } + TupleDescFinalize(attributeDescriptor); + return attributeDescriptor; } @@ -1417,6 +1543,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..6d62ec57a 100644 --- a/pg_lake_copy/tests/pytests/test_parquet_copy.py +++ b/pg_lake_copy/tests/pytests/test_parquet_copy.py @@ -6,6 +6,11 @@ from utils_pytest import * +def _skip_if_partitioned_copy_unsupported(conn): + if get_pg_version_num(conn) < 190000: + 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 +711,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_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_if_partitioned_copy_unsupported(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_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_if_partitioned_copy_unsupported(pg_conn) + + 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( 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_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_if_partitioned_copy_unsupported(pg_conn) + + out_url = f"s3://{TEST_BUCKET}/test_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_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_if_partitioned_copy_unsupported(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_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_if_partitioned_copy_unsupported(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..beeae12d4 --- /dev/null +++ b/pg_lake_copy/tests/pytests/test_pg19_copy_options.py @@ -0,0 +1,120 @@ +"""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 * + + +def _skip_pre_pg19(conn): + if get_pg_version_num(conn) < 190000: + 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..8558ce0b1 100644 --- a/pg_lake_engine/src/query/execute.c +++ b/pg_lake_engine/src/query/execute.c @@ -75,7 +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 + 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); } 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/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" 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..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,7 +22,7 @@ #include "pg_lake/util/s3_reader_utils.h" #include "nodes/pg_list.h" -#include "utils/dynahash.h" +#include "utils/hsearch.h" #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..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 @@ -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; + /* + * 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); + + columnMapping->pgType = MakePGType(attr->atttypid, attr->atttypmod); + columnMapping->attNotNull = attr->attnotnull; + columnMapping->attHasDef = attr->atthasdef; + } 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..2b599d639 --- /dev/null +++ b/pg_lake_table/tests/pytests/test_pg19_new_features.py @@ -0,0 +1,916 @@ +"""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 * + + +def _skip_if_not_pg19(conn): + if get_pg_version_num(conn) < 190000: + 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 _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, + 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() + + 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}" + + # 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): + """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..b94562f53 --- /dev/null +++ b/pgduck_server/tests/pytests/test_protocol_negotiation.py @@ -0,0 +1,161 @@ +"""Wire-protocol coverage for pgduck_server's NegotiateProtocolVersion handling. + +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), + * 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")