pg_lake_copy: reject multidimensional arrays in COPY TO before CSV serialization#428
Open
sfc-gh-rbachala wants to merge 1 commit into
Open
Conversation
…rialization COPY <table> TO '<lake-file>' 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 Snowflake-Labs#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 Snowflake-Labs#407. Mitigates Snowflake-Labs#408 (multidim values are now blocked before reaching DuckDB). Signed-off-by: Richie Bachala <richie.bachala@snowflake.com>
sfc-gh-rbachala
force-pushed
the
fix/407-copy-to-multidim-guardrail
branch
from
50b822d to
99f3977
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
COPY <table> TO '<lake-file>'(Parquet, JSON, CSV-to-URL).IcebergErrorOrClampMultiDimArrayDatuminiceberg_datum_validation.c. PlainCOPY TOhad no equivalent — multidim values flowed into the intermediate temp CSV and reached DuckDB, which either returned a crypticConversion Erroror crashed the sharedpgduck_serverprocess (issue DuckDB : CSV string-to-LIST cast can segfault (standalone repro) #408), disconnecting all concurrent sessions.CopyOneRowTo(csv_writer.c): after the existing numeric NaN/Inf checks, detect array-typed columns and raiseERRCODE_FEATURE_NOT_SUPPORTEDwhenARR_NDIM > 1, before the value is ever serialised to the CSV. The check is skipped forDATA_FORMAT_ICEBERGwhere the upstream guardrail already applies.ConvertCSVFileTo(write_data.c) that implied clamping had occurred for all callers; it is only true for the Iceberg FDW INSERT path.Root Cause
ProcessPgLakeCopyTo(copy.c:901) callsCreateCSVDestReceiver→ExecuteQueryToDestReceiverwith no pre-write validation.CopyOneRowToserializes each attribute directly. A column declaredint[]can holdARRAY[[1,2],[3,4]]at runtime (PostgreSQL cannot distinguishint[]fromint[][]at the type level), which serialises to"[[1,2],[3,4]]"in the CSV. DuckDB cannot cast that string back toINTEGER[]inStringValueScanner::Flush.Scope
The check is added once in
CopyOneRowToand covers all callers ofCreateCSVDestReceiverthat handle user array data. Other callers (position-delete dest, multi-file Iceberg dest) already have upstream validation or carry no user array data.Tests
test_copy_to_multidim_array_errors: table with a multidimint[]value → COPY TO parquet → verifies"multidimensional arrays are not supported in COPY TO"error is raised before any engine interactiontest_copy_to_1d_array_succeeds: 1-Dtext[]column → COPY TO parquet → verifies values round-trip correctly (regression guard)Notes
pgindentis not available locally; the C change adds one#include, one comment block, and oneifblock. CI should confirm formatting. This also mitigates issue #408 — multidim values are now blocked at the PG level before reaching DuckDB.Fixes #407.
Mitigates #408.