Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions pg_lake_copy/tests/pytests/test_csv_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
102 changes: 102 additions & 0 deletions pg_lake_copy/tests/pytests/test_parquet_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,108 @@ 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 <table> TO <lake-file> 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_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:
Expand Down
51 changes: 50 additions & 1 deletion pg_lake_engine/src/csv/csv_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -770,7 +771,55 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
*/
Form_pg_attribute attr = TupleDescAttr(slot->tts_tupleDescriptor, attnum - 1);

if (ShouldUseDuckSerialization(cstate->targetFormat, MakePGType(attr->atttypid, attr->atttypmod)))
bool useDuckSerialization =
ShouldUseDuckSerialization(cstate->targetFormat,
MakePGType(attr->atttypid, attr->atttypmod));

/*
* 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 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 Parquet or JSON file.
*/
if (useDuckSerialization &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also disallow in arbitrarily nested structures? (can recurse as in #448 )

get_element_type(attr->atttypid) != InvalidOid)
{
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, write to CSV format, or write"
" to an Iceberg table with"
" out_of_range_values = 'clamp'.")));
}

if (useDuckSerialization)
{
/*
* Since we are at the top-level when emitting an
Expand Down
11 changes: 9 additions & 2 deletions pg_lake_engine/src/pgduck/write_data.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,15 @@ 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) 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,
Expand Down