pg_lake_copy: reject multidimensional arrays in COPY TO before CSV serialization#430
Open
sfc-gh-mslot wants to merge 1 commit into
Open
pg_lake_copy: reject multidimensional arrays in COPY TO before CSV serialization#430sfc-gh-mslot wants to merge 1 commit into
sfc-gh-mslot wants to merge 1 commit into
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 #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 <richie.bachala@snowflake.com> Signed-off-by: Marco Slot <marco.slot@snowflake.com>
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
2 participants
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.
Rebased version of #428 (by @sfc-gh-rbachala) with the C indentation corrected, opened to run CI.
The original change is unmodified in substance. The only fix here is running
pgindentover the two edited C files: the added block inCopyOneRowToand the rewritten comment inConvertCSVFileToused space indentation, which fails thecheck-indentCI step. Reindenting restores the project's tab style, so the diff against main is now just the logical change (106 insertions, 2 deletions).Authorship is preserved: the commit is still authored by Richie Bachala with their
Signed-off-by, with my own sign-off added as committer.What the change does
COPY
<table>TO a lake file serializes rows through an intermediate CSV before handing them to DuckDB. That path had no guard against multidimensional array values, so a column declaredint[]holdingARRAY[[1,2],[3,4]]would serialize to[[1,2],[3,4]], which DuckDB cannot cast back to a flatLIST(T)— returning a cryptic Conversion Error or, for some element types, crashing the shared pgduck_server process (#408). The fix rejectsARR_NDIM > 1inCopyOneRowTobefore serialization, matching the guard the Iceberg write path already has.Fixes #407. Mitigates #408.
Test plan
test_copy_to_multidim_array_errors— a multidimint[]value raises a clear pg_lake error with no engine involvementtest_copy_to_1d_array_succeeds— 1-D arrays still round-trip correctlyCI on this branch exercises the full build, indentation check, and pytest suite.