From 8022809a91f2ca3a21173fc8b0978378db9d24db Mon Sep 17 00:00:00 2001 From: Richie Bachala Date: Fri, 3 Jul 2026 16:53:07 -0700 Subject: [PATCH 1/3] pg_lake_copy: reject multidimensional arrays in COPY TO before CSV serialization COPY TO '' serializes rows to an intermediate temp CSV and hands it to DuckDB for conversion to Parquet/JSON. The intermediate CSV path had no guard against multidimensional array values: a column declared int[] can hold ARRAY[[1,2],[3,4]] at runtime, which serializes to [[1,2],[3,4]] in the CSV. DuckDB cannot cast that string back to a flat LIST(T) and either returns a cryptic Conversion Error or -- for certain element types (uuid, bigint, numeric) -- crashes the shared pgduck_server process, disconnecting all concurrent sessions (issue #408). The Iceberg table write path already handles this via IcebergErrorOrClampMultiDimArrayDatum in iceberg_datum_validation.c; plain COPY TO had no equivalent guard. Fix: in CopyOneRowTo (csv_writer.c), after the existing numeric NaN/Inf checks, detect array-typed columns and raise ERRCODE_FEATURE_NOT_SUPPORTED when ARR_NDIM > 1, before the value is serialized to the CSV. The check is skipped for the Iceberg format (DATA_FORMAT_ICEBERG) where the upstream guardrail already applies. Also corrects a misleading comment in ConvertCSVFileTo (write_data.c) that described the ICEBERG_OOR_NONE sentinel as if clamping had already occurred for all callers; it is only true for the Iceberg FDW INSERT path. Tests: - test_copy_to_multidim_array_errors: verifies a clear pg_lake error is raised for a table with a multidim int[] value, with no engine involved - test_copy_to_1d_array_succeeds: regression guard confirming 1-D arrays still round-trip correctly after the check is added Fixes #407. Mitigates #408 (multidim values are now blocked before reaching DuckDB). Signed-off-by: Richie Bachala Signed-off-by: Marco Slot --- .../tests/pytests/test_parquet_copy.py | 70 +++++++++++++++++++ pg_lake_engine/src/csv/csv_writer.c | 31 ++++++++ pg_lake_engine/src/pgduck/write_data.c | 7 +- 3 files changed, 106 insertions(+), 2 deletions(-) diff --git a/pg_lake_copy/tests/pytests/test_parquet_copy.py b/pg_lake_copy/tests/pytests/test_parquet_copy.py index 48be82fe..8ffce6ee 100644 --- a/pg_lake_copy/tests/pytests/test_parquet_copy.py +++ b/pg_lake_copy/tests/pytests/test_parquet_copy.py @@ -884,6 +884,76 @@ def test_md_array(pg_conn, duckdb_conn, tmp_path): assert result[0]["val"] == 2 +def test_copy_to_multidim_array_errors(pg_conn, tmp_path): + """ + Regression test for https://github.com/Snowflake-Labs/pg_lake/issues/407. + + COPY
TO must raise a clear pg_lake error when a column + contains a multidimensional array value. Previously the value was + serialised into the intermediate temp CSV and handed to DuckDB, which + either returned a cryptic Conversion Error or crashed the shared + pgduck_server process (issue #408). + + The check lives in CopyOneRowTo (csv_writer.c) and fires before the CSV + is written, so the engine is never involved. + """ + parquet_path = tmp_path / "test_multidim_err.parquet" + + run_command( + """ + CREATE TABLE test_multidim_err (id bigint, v int[]); + INSERT INTO test_multidim_err VALUES (1, ARRAY[[1,2],[3,4]]); + """, + pg_conn, + ) + + error = run_command( + f"COPY test_multidim_err TO '{parquet_path}' WITH (format 'parquet')", + pg_conn, + raise_error=False, + ) + + assert error is not None, "Expected an error for multidimensional array in COPY TO" + assert ( + "multidimensional arrays are not supported" in error.lower() + ), f"Unexpected error message: {error}" + + pg_conn.rollback() + + +def test_copy_to_1d_array_succeeds(pg_conn, duckdb_conn, tmp_path): + """ + Regression guard: 1-D array columns must still round-trip correctly through + COPY TO after the multidim check is added. A 1-D value has ARR_NDIM == 1 + and must not be rejected. + """ + parquet_path = tmp_path / "test_1d_array.parquet" + + run_command( + f""" + CREATE TABLE test_1d_array (id bigint, tags text[]); + INSERT INTO test_1d_array VALUES + (1, ARRAY['a', 'b', 'c']), + (2, NULL), + (3, ARRAY['x']); + COPY test_1d_array TO '{parquet_path}' WITH (format 'parquet'); + """, + pg_conn, + ) + + duckdb_conn.execute( + "SELECT id, tags FROM read_parquet($1) ORDER BY id", [str(parquet_path)] + ) + rows = duckdb_conn.fetchall() + + assert len(rows) == 3 + assert rows[0] == (1, ["a", "b", "c"]) + assert rows[1] == (2, None) + assert rows[2] == (3, ["x"]) + + pg_conn.rollback() + + def test_copy_virtual_column(pg_conn, tmp_path): # virtual columns were introduced in PostgreSQL 18 if get_pg_version_num(pg_conn) < 180000: diff --git a/pg_lake_engine/src/csv/csv_writer.c b/pg_lake_engine/src/csv/csv_writer.c index 3b91870f..f0945560 100644 --- a/pg_lake_engine/src/csv/csv_writer.c +++ b/pg_lake_engine/src/csv/csv_writer.c @@ -43,6 +43,7 @@ #include "nodes/execnodes.h" #include "nodes/makefuncs.h" #include "port/pg_bswap.h" +#include "utils/array.h" #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -770,6 +771,36 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) */ Form_pg_attribute attr = TupleDescAttr(slot->tts_tupleDescriptor, attnum - 1); + /* + * Reject multidimensional arrays before serialization. + * PostgreSQL cannot distinguish int[] from int[][] at the + * type level, so a value with ndim > 1 would be serialised as + * "[[1,2],[3,4]]" and DuckDB cannot cast that string back to + * a flat LIST(T). For Iceberg tables this is handled + * upstream by IcebergErrorOrClampDatum; for plain COPY TO + * there is no such guard, so we raise here. Check before + * serialisation so we do not pay the cost of PGDuckSerialize + * on a value we will reject. + */ + if (get_element_type(attr->atttypid) != InvalidOid && + cstate->targetFormat != DATA_FORMAT_ICEBERG) + { + ArrayType *arr = DatumGetArrayTypeP(value); + + if (ARR_NDIM(arr) > 1) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("multidimensional arrays are not supported" + " in COPY TO"), + errdetail("Column \"%s\" contains a" + " %d-dimensional array value.", + NameStr(attr->attname), + ARR_NDIM(arr)), + errhint("Flatten the array to one dimension before" + " exporting, or write to an Iceberg table" + " with out_of_range_values = 'clamp'."))); + } + if (ShouldUseDuckSerialization(cstate->targetFormat, MakePGType(attr->atttypid, attr->atttypmod))) { /* diff --git a/pg_lake_engine/src/pgduck/write_data.c b/pg_lake_engine/src/pgduck/write_data.c index ff333a9b..b4d9c7c7 100644 --- a/pg_lake_engine/src/pgduck/write_data.c +++ b/pg_lake_engine/src/pgduck/write_data.c @@ -93,8 +93,11 @@ ConvertCSVFileTo(char *csvFilePath, TupleDesc csvTupleDesc, int maxLineSize, bool queryHasRowIds = false; /* - * CSV data is already clamped by WriteInsertRecord and converted to - * struct for Iceberg + * When reached from the Iceberg FDW INSERT path, the CSV data has already + * been clamped by WriteInsertRecord (via ClampAndCheckConstraints → + * IcebergErrorOrClampSlotInPlace). When reached from the plain COPY TO + * path (ProcessPgLakeCopyTo), multidimensional array values are rejected + * earlier in CopyOneRowTo, so they never appear in the CSV. */ return WriteQueryResultTo(command.data, destinationPath, From 41e33ce3e42074d47a1b7cf601f631ef6ab68241 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Fri, 17 Jul 2026 08:21:57 +0000 Subject: [PATCH 2/3] Address review: drop redundant DATA_FORMAT_ICEBERG guard on multidim check The multidimensional array check in CopyOneRowTo skipped DATA_FORMAT_ICEBERG, but no reachable path delivers a multidimensional value to the CSV writer with that target format: COPY TO in Iceberg format is rejected earlier in EnsureFormatSupported, and the Iceberg INSERT path already rejects or nullifies such values upstream via IcebergErrorOrClampDatum. Running the check for every format is a harmless backstop and simpler. Comment updated to match. Signed-off-by: Marco Slot --- pg_lake_engine/src/csv/csv_writer.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/pg_lake_engine/src/csv/csv_writer.c b/pg_lake_engine/src/csv/csv_writer.c index f0945560..2883f1c6 100644 --- a/pg_lake_engine/src/csv/csv_writer.c +++ b/pg_lake_engine/src/csv/csv_writer.c @@ -776,14 +776,19 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) * PostgreSQL cannot distinguish int[] from int[][] at the * type level, so a value with ndim > 1 would be serialised as * "[[1,2],[3,4]]" and DuckDB cannot cast that string back to - * a flat LIST(T). For Iceberg tables this is handled - * upstream by IcebergErrorOrClampDatum; for plain COPY TO - * there is no such guard, so we raise here. Check before - * serialisation so we do not pay the cost of PGDuckSerialize - * on a value we will reject. + * a flat LIST(T). Check before serialisation so we do not + * pay the cost of PGDuckSerialize on a value we will reject. + * + * This runs for every target format as a backstop. For the + * Iceberg write path (INSERT into an Iceberg table) a + * multidimensional value is already rejected (error policy) + * or set to NULL (clamp policy) upstream by + * IcebergErrorOrClampDatum, so it never reaches here with + * ndim > 1. COPY TO in Iceberg format is rejected earlier in + * EnsureFormatSupported. In practice this only fires on + * plain COPY TO to a file. */ - if (get_element_type(attr->atttypid) != InvalidOid && - cstate->targetFormat != DATA_FORMAT_ICEBERG) + if (get_element_type(attr->atttypid) != InvalidOid) { ArrayType *arr = DatumGetArrayTypeP(value); From 5621ac232e47a08e1089c2970d18f2e5e6641495 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Fri, 17 Jul 2026 09:01:48 +0000 Subject: [PATCH 3/3] pg_lake_copy: exempt CSV from the COPY TO multidim-array guard The multidimensional-array guard added in CopyOneRowTo ran for every target format. That was broader than the bug it fixes: only formats that serialize the value through DuckDB as a typed LIST(T) (Parquet/Iceberg/ JSON) hit the failing cast (#407/#408). For CSV, ShouldUseDuckSerialization is false and ChooseDuckDBEngineTypeForWrite treats every column as VARCHAR, so a value is written using PostgreSQL array text ("{{1,2},{3,4}}") and copied through verbatim -- multidimensional arrays round-trip fine today. Rejecting them for CSV was therefore a behaviour regression for COPY
TO '' WITH (format 'csv'). Gate the guard on useDuckSerialization so it fires only for the formats that actually cast to a list, and update the errhint to mention CSV as an escape hatch. Regression coverage: - test_copy_to_multidim_array_csv (test_csv_copy.py): multidim int[] to a CSV object-store file succeeds and is preserved as PG array text. - test_copy_to_multidim_array_json_errors (test_parquet_copy.py): JSON still rejects multidim arrays, locking in the guard for DuckDB-serialized formats. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Marco Slot --- pg_lake_copy/tests/pytests/test_csv_copy.py | 46 +++++++++++++++++++ .../tests/pytests/test_parquet_copy.py | 32 +++++++++++++ pg_lake_engine/src/csv/csv_writer.c | 29 ++++++++---- pg_lake_engine/src/pgduck/write_data.c | 8 +++- 4 files changed, 105 insertions(+), 10 deletions(-) diff --git a/pg_lake_copy/tests/pytests/test_csv_copy.py b/pg_lake_copy/tests/pytests/test_csv_copy.py index 35044d45..c4c41ee5 100644 --- a/pg_lake_copy/tests/pytests/test_csv_copy.py +++ b/pg_lake_copy/tests/pytests/test_csv_copy.py @@ -1074,3 +1074,49 @@ def test_auto_detect(pg_conn, azure): # assert result_before == result_after pg_conn.rollback() + + +def test_copy_to_multidim_array_csv(pg_conn, s3): + """ + Regression test for https://github.com/Snowflake-Labs/pg_lake/issues/407. + + Unlike Parquet/JSON, COPY TO in CSV format must still ACCEPT + multidimensional arrays. For CSV, ShouldUseDuckSerialization is false and + ChooseDuckDBEngineTypeForWrite treats every column as VARCHAR, so the value + is written using the PostgreSQL text representation ("{{1,2},{3,4}}") and + copied through verbatim rather than cast to a DuckDB LIST(T). The multidim + guard in CopyOneRowTo must therefore NOT fire for CSV, otherwise a value + that writes fine today would start erroring. + + (Local-file / STDOUT CSV is handled by PostgreSQL directly and never + reaches pg_lake, so we exercise the pg_lake CSV path via an object-store + URL.) + """ + csv_key = "test_copy_to_multidim_array_csv/data.csv" + csv_path = f"s3://{TEST_BUCKET}/{csv_key}" + + # Must not raise: the guard is scoped to DuckDB-serialised formats. + run_command( + f""" + CREATE TABLE test_multidim_csv (id bigint, v int[]); + INSERT INTO test_multidim_csv VALUES + (1, ARRAY[[1,2],[3,4]]), + (2, ARRAY[10,20,30]), + (3, NULL); + COPY test_multidim_csv TO '{csv_path}' WITH (format 'csv'); + """, + pg_conn, + ) + + # The multidimensional value is preserved verbatim as PostgreSQL array + # text (the field is quoted because it contains commas, but the literal is + # intact), and the 1-D array is written alongside it. + content = s3.get_object(Bucket=TEST_BUCKET, Key=csv_key)["Body"].read().decode() + assert ( + "{{1,2},{3,4}}" in content + ), f"multidimensional array text missing from CSV output: {content!r}" + assert ( + "{10,20,30}" in content + ), f"1-D array text missing from CSV output: {content!r}" + + 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 8ffce6ee..6596bbaf 100644 --- a/pg_lake_copy/tests/pytests/test_parquet_copy.py +++ b/pg_lake_copy/tests/pytests/test_parquet_copy.py @@ -954,6 +954,38 @@ def test_copy_to_1d_array_succeeds(pg_conn, duckdb_conn, tmp_path): pg_conn.rollback() +def test_copy_to_multidim_array_json_errors(pg_conn, tmp_path): + """ + Companion to the CSV case: JSON serialises arrays through DuckDB as a + typed LIST(T) (ShouldUseDuckSerialization is true), so a multidimensional + value must still be rejected by the guard in CopyOneRowTo. + """ + json_path = tmp_path / "test_multidim.json" + + run_command( + """ + CREATE TABLE test_multidim_json (id bigint, v int[]); + INSERT INTO test_multidim_json VALUES (1, ARRAY[[1,2],[3,4]]); + """, + pg_conn, + ) + + error = run_command( + f"COPY test_multidim_json TO '{json_path}' WITH (format 'json')", + pg_conn, + raise_error=False, + ) + + assert ( + error is not None + ), "Expected an error for multidimensional array in COPY TO json" + assert ( + "multidimensional arrays are not supported" in error.lower() + ), f"Unexpected error message: {error}" + + pg_conn.rollback() + + def test_copy_virtual_column(pg_conn, tmp_path): # virtual columns were introduced in PostgreSQL 18 if get_pg_version_num(pg_conn) < 180000: diff --git a/pg_lake_engine/src/csv/csv_writer.c b/pg_lake_engine/src/csv/csv_writer.c index 2883f1c6..766036a0 100644 --- a/pg_lake_engine/src/csv/csv_writer.c +++ b/pg_lake_engine/src/csv/csv_writer.c @@ -771,24 +771,36 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) */ Form_pg_attribute attr = TupleDescAttr(slot->tts_tupleDescriptor, attnum - 1); + bool useDuckSerialization = + ShouldUseDuckSerialization(cstate->targetFormat, + MakePGType(attr->atttypid, attr->atttypmod)); + /* - * Reject multidimensional arrays before serialization. + * Reject multidimensional arrays before DuckDB serialization. * PostgreSQL cannot distinguish int[] from int[][] at the * type level, so a value with ndim > 1 would be serialised as * "[[1,2],[3,4]]" and DuckDB cannot cast that string back to * a flat LIST(T). Check before serialisation so we do not * pay the cost of PGDuckSerialize on a value we will reject. * - * This runs for every target format as a backstop. For the - * Iceberg write path (INSERT into an Iceberg table) a + * This only fires for formats that hand the value to DuckDB + * as a typed LIST(T) (Parquet/Iceberg/JSON), i.e. exactly the + * formats for which ShouldUseDuckSerialization returns true + * for an array. CSV is deliberately exempt: it preserves the + * PostgreSQL text representation ("{{1,2},{3,4}}") and reads + * every column back as VARCHAR, so a multidimensional value + * round-trips faithfully and must not be rejected (#407). + * + * For the Iceberg write path (INSERT into an Iceberg table) a * multidimensional value is already rejected (error policy) * or set to NULL (clamp policy) upstream by * IcebergErrorOrClampDatum, so it never reaches here with * ndim > 1. COPY TO in Iceberg format is rejected earlier in * EnsureFormatSupported. In practice this only fires on - * plain COPY TO to a file. + * plain COPY TO to a Parquet or JSON file. */ - if (get_element_type(attr->atttypid) != InvalidOid) + if (useDuckSerialization && + get_element_type(attr->atttypid) != InvalidOid) { ArrayType *arr = DatumGetArrayTypeP(value); @@ -802,11 +814,12 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) NameStr(attr->attname), ARR_NDIM(arr)), errhint("Flatten the array to one dimension before" - " exporting, or write to an Iceberg table" - " with out_of_range_values = 'clamp'."))); + " exporting, write to CSV format, or write" + " to an Iceberg table with" + " out_of_range_values = 'clamp'."))); } - if (ShouldUseDuckSerialization(cstate->targetFormat, MakePGType(attr->atttypid, attr->atttypmod))) + if (useDuckSerialization) { /* * Since we are at the top-level when emitting an diff --git a/pg_lake_engine/src/pgduck/write_data.c b/pg_lake_engine/src/pgduck/write_data.c index b4d9c7c7..3cb49211 100644 --- a/pg_lake_engine/src/pgduck/write_data.c +++ b/pg_lake_engine/src/pgduck/write_data.c @@ -96,8 +96,12 @@ ConvertCSVFileTo(char *csvFilePath, TupleDesc csvTupleDesc, int maxLineSize, * When reached from the Iceberg FDW INSERT path, the CSV data has already * been clamped by WriteInsertRecord (via ClampAndCheckConstraints → * IcebergErrorOrClampSlotInPlace). When reached from the plain COPY TO - * path (ProcessPgLakeCopyTo), multidimensional array values are rejected - * earlier in CopyOneRowTo, so they never appear in the CSV. + * path (ProcessPgLakeCopyTo) with a DuckDB-serialised destination + * (Parquet/JSON), multidimensional array values are rejected earlier in + * CopyOneRowTo, so they never appear in the CSV. For a CSV destination + * such values may appear, but every column is read back as VARCHAR (see + * ChooseDuckDBEngineTypeForWrite), so the PostgreSQL array text is copied + * through verbatim rather than cast to a LIST(T). */ return WriteQueryResultTo(command.data, destinationPath,