Skip to content

Commit 521f521

Browse files
Use SPI parameter binding for composite-type catalog lookup
GetOrCreatePGStructType was psprintf-ing composite field names directly into a '{...}'::name[] literal. Hostile shapes (commas, colons, quotes, spaces, backslashes, parens in field names) produced malformed array literals and a confusing SPI error on auto-detect. Switch to SPI_execute_with_args with the names and oids passed as properly constructed name[]/oid[] arrays. Adds a regression test covering an auto-detect path through CREATE FOREIGN TABLE () against a parquet file whose struct fields contain each hostile character class. Addresses review feedback on PR #297. Signed-off-by: David Christensen <david.christensen@snowflake.com>
1 parent 65fb677 commit 521f521

2 files changed

Lines changed: 121 additions & 28 deletions

File tree

pg_lake_engine/src/pgduck/parse_struct.c

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "commands/typecmds.h"
4141
#include "executor/spi.h"
4242
#include "nodes/makefuncs.h"
43+
#include "utils/array.h"
4344
#include "utils/builtins.h"
4445
#include "utils/lsyscache.h"
4546
#include "utils/syscache.h"
@@ -734,51 +735,58 @@ FindOrCreatePGCompositeType(CompositeType * type)
734735
*/
735736
do
736737
{
737-
StringInfo oidArray = makeStringInfo();
738-
StringInfo nameArray = makeStringInfo();
738+
int ncols = list_length(type->cols);
739+
Datum *nameDatums = palloc(sizeof(Datum) * ncols);
740+
Datum *oidDatums = palloc(sizeof(Datum) * ncols);
739741
ListCell *lc;
742+
int i = 0;
740743

741744
foreach(lc, type->cols)
742745
{
743746
CompositeCol *col = (CompositeCol *) lfirst(lc);
744747

745748
Assert(col->colType);
746749

747-
/* if we are the first time through, skip this */
748-
if (oidArray->len)
749-
{
750-
appendStringInfoChar(oidArray, ',');
751-
appendStringInfoChar(nameArray, ',');
752-
}
753-
754-
appendStringInfo(oidArray, "%d", col->colType);
755-
appendStringInfo(nameArray, "%s", col->colName);
750+
/*
751+
* Build the arrays as proper Datums rather than string-formatting
752+
* into a name[] literal: field names may contain commas, quotes,
753+
* backslashes, or whitespace that would break array literal
754+
* parsing.
755+
*/
756+
nameDatums[i] = DirectFunctionCall1(namein,
757+
CStringGetDatum(col->colName));
758+
oidDatums[i] = ObjectIdGetDatum(col->colType);
759+
i++;
756760
}
757761

758-
char *findTypeQuery = psprintf(
759-
"SELECT oid, typname, typarray FROM pg_type JOIN "
760-
"(SELECT attrelid,"
761-
" array_agg(atttypid ORDER BY attnum) AS types,"
762-
" array_agg(attname ORDER BY attnum) AS names "
763-
" FROM pg_attribute "
764-
" WHERE attnum > 0 AND NOT attisdropped "
765-
" GROUP BY attrelid) atts "
766-
"ON pg_type.typrelid = atts.attrelid AND "
767-
" atts.names = '{%s}'::name[] and atts.types='{%s}'::oid[] "
768-
"WHERE typnamespace :: regnamespace in ('pg_catalog','"
769-
STRUCT_TYPES_SCHEMA "') LIMIT 1",
770-
nameArray->data,
771-
oidArray->data);
772-
773-
elog(DEBUG1, "checking for matching type oid via query: %s", findTypeQuery);
762+
ArrayType *nameArr = construct_array_builtin(nameDatums, ncols, NAMEOID);
763+
ArrayType *oidArr = construct_array_builtin(oidDatums, ncols, OIDOID);
764+
765+
const char *findTypeQuery =
766+
"SELECT oid, typname, typarray FROM pg_type JOIN "
767+
"(SELECT attrelid,"
768+
" array_agg(atttypid ORDER BY attnum) AS types,"
769+
" array_agg(attname ORDER BY attnum) AS names "
770+
" FROM pg_attribute "
771+
" WHERE attnum > 0 AND NOT attisdropped "
772+
" GROUP BY attrelid) atts "
773+
"ON pg_type.typrelid = atts.attrelid AND "
774+
" atts.names = $1 AND atts.types = $2 "
775+
"WHERE typnamespace :: regnamespace in "
776+
" ('pg_catalog','" STRUCT_TYPES_SCHEMA "') LIMIT 1";
777+
778+
Oid argTypes[2] = {NAMEARRAYOID, OIDARRAYOID};
779+
Datum argValues[2] = {PointerGetDatum(nameArr),
780+
PointerGetDatum(oidArr)};
774781

775782
Oid foundOid = InvalidOid;
776783
Oid foundArrayOid = InvalidOid;
777784
char *foundName = NULL;
778785

779786
SPI_START();
780787

781-
int ret = SPI_execute(findTypeQuery, false, 1);
788+
int ret = SPI_execute_with_args(findTypeQuery, 2, argTypes,
789+
argValues, NULL, false, 1);
782790

783791
if (ret == SPI_OK_SELECT && SPI_processed >= 1)
784792
{

pg_lake_table/tests/pytests/test_duckdb_reserved_keywords.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,91 @@ def test_each_reserved_keyword_csv(pg_conn, s3, extension, keyword):
497497
# ---------------------------------------------------------------------------
498498

499499

500+
def test_autodetect_struct_field_names_with_hostile_characters(pg_conn, s3, extension):
501+
"""
502+
Auto-detect (CREATE FOREIGN TABLE () ... OPTIONS (path ...)) against a
503+
parquet file whose struct field names contain characters that would break
504+
a raw-formatted name[] array literal: quotes, commas, colons, spaces,
505+
parens, backslashes.
506+
507+
Regression test for review feedback on PR #297 — after fixing
508+
ParseDuckDBFieldName to handle SQL-standard "" doubling, the next layer
509+
(GetOrCreatePGStructType's catalog-lookup SPI query) was still formatting
510+
field names directly into a '{...}'::name[] literal, producing
511+
"malformed array literal" for any hostile shape.
512+
"""
513+
schema = "test_duckdb_kw_autodetect_hostile"
514+
url = f"s3://{TEST_BUCKET}/{schema}/data.parquet"
515+
516+
run_command(f"CREATE SCHEMA {schema}", pg_conn)
517+
pg_conn.commit()
518+
519+
run_command(
520+
f"""
521+
CREATE TYPE {schema}.edge_t AS (
522+
plain int,
523+
"with space" int,
524+
U&"has\\0022one" int,
525+
"comma,here" int,
526+
"colon:here" int,
527+
"with(parens)" int,
528+
U&"back\\005Cslash" int
529+
)
530+
""",
531+
pg_conn,
532+
)
533+
pg_conn.commit()
534+
535+
run_command(
536+
f"""
537+
CREATE TABLE {schema}.src (id int, e {schema}.edge_t)
538+
USING iceberg
539+
WITH (location = 's3://{TEST_BUCKET}/{schema}_src/')
540+
""",
541+
pg_conn,
542+
)
543+
pg_conn.commit()
544+
545+
run_command(
546+
f"INSERT INTO {schema}.src VALUES (1, ROW(1, 2, 3, 4, 5, 6, 7))",
547+
pg_conn,
548+
)
549+
pg_conn.commit()
550+
551+
run_command(
552+
f"COPY (SELECT * FROM {schema}.src) TO '{url}' WITH (format 'parquet')",
553+
pg_conn,
554+
)
555+
pg_conn.commit()
556+
557+
# Drop the catalog-side composite type so the next CREATE FOREIGN TABLE is
558+
# forced down the auto-detect path through GetOrCreatePGStructType.
559+
run_command(f"DROP TABLE {schema}.src", pg_conn)
560+
run_command(f"DROP TYPE {schema}.edge_t", pg_conn)
561+
pg_conn.commit()
562+
563+
run_command(
564+
f"""
565+
CREATE FOREIGN TABLE {schema}.edge_pq ()
566+
SERVER pg_lake
567+
OPTIONS (path '{url}')
568+
""",
569+
pg_conn,
570+
)
571+
pg_conn.commit()
572+
573+
result = run_query(
574+
f'SELECT id, (e).plain, (e)."with space", (e).U&"has\\0022one", '
575+
f'(e)."comma,here", (e)."colon:here", (e)."with(parens)", '
576+
f'(e).U&"back\\005Cslash" FROM {schema}.edge_pq',
577+
pg_conn,
578+
)
579+
assert result == [[1, 1, 2, 3, 4, 5, 6, 7]]
580+
581+
run_command(f"DROP SCHEMA {schema} CASCADE", pg_conn)
582+
pg_conn.commit()
583+
584+
500585
def test_iceberg_composite_field_with_embedded_double_quote(pg_conn, s3, extension):
501586
"""
502587
Composite type field names containing double-quote characters must be

0 commit comments

Comments
 (0)