Skip to content

Commit 3c29903

Browse files
Fix DuckDB reserved keyword quoting for column names
Replace the fragile RequoteDuckDBReservedInSQL() string scanner with PostgreSQL's built-in quote_all_identifiers mechanism. Setting this flag before calling pg_get_querydef() causes all identifiers to be double-quoted, which covers DuckDB-reserved words without needing a separate post-processing pass. Update all test assertions that check deparsed SQL fragments to expect the now-quoted identifiers (function names, column names, field access). Retain duckdb_quote_identifier() for direct SQL building paths (e.g., read_data.c, write_data.c) that don't go through pg_get_querydef. Signed-off-by: David Christensen <david.christensen@snowflake.com>
1 parent aec27fe commit 3c29903

52 files changed

Lines changed: 3906 additions & 803 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Makefile

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ CUSTOM_TARGETS = check-pg_lake_engine installcheck-pg_lake_engine check-pg_exten
1414
DUCKDB_BUILD_USE_CACHE ?= 0
1515

1616
# other phony targets go here
17-
.PHONY: all fast install install-fast installcheck clean check submodules uninstall check-indent reindent installcheck-postgres installcheck-postgres-with_extensions_created
17+
.PHONY: all fast install install-fast installcheck clean check submodules uninstall check-indent reindent installcheck-postgres installcheck-postgres-with_extensions_created generate-duckdb-kwlist check-duckdb-kwlist
1818
.PHONY: $(ALL_TARGETS)
1919
.PHONY: $(PHONY_TARGETS)
2020

@@ -222,6 +222,17 @@ uninstall-avro:
222222
rm -f $(PG_LIBDIR)/libavro.*
223223
rm -rf $(PG_INCLUDEDIR)/avro*
224224

225+
## DuckDB keyword list maintenance
226+
# Regenerate the checked-in keyword table from the vendored DuckDB kwlist.hpp.
227+
# Re-run whenever duckdb_pglake/duckdb is updated to a new DuckDB release.
228+
generate-duckdb-kwlist:
229+
python3 tools/generate_duckdb_kwlist.py
230+
231+
# Verify that the checked-in keyword table matches the current kwlist.hpp.
232+
# Run in CI to catch stale keyword tables after a DuckDB version bump.
233+
check-duckdb-kwlist:
234+
python3 tools/generate_duckdb_kwlist.py --check
235+
225236
## Other targets
226237
check-isolation_pg_lake_table:
227238
$(MAKE) -C pg_lake_table check-isolation
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
"""
2+
Tests for correct identifier quoting during COPY TO / COPY FROM when table
3+
columns are named after DuckDB reserved keywords.
4+
5+
DuckDB reserves several keywords (PIVOT, QUALIFY, LAMBDA, etc.) that
6+
PostgreSQL does not. These are legal unquoted column names in PostgreSQL but
7+
require quoting in the SQL that pg_lake generates for pgduck_server.
8+
9+
See: https://github.com/Snowflake-Labs/pg_lake/issues/277
10+
"""
11+
12+
import pytest
13+
from utils_pytest import *
14+
15+
# DuckDB-only reserved keywords that are safe as unquoted PostgreSQL
16+
# identifiers.
17+
DUCKDB_ONLY_RESERVED = [
18+
"describe",
19+
"lambda",
20+
"pivot",
21+
"pivot_longer",
22+
"pivot_wider",
23+
"qualify",
24+
"show",
25+
"summarize",
26+
"unpivot",
27+
]
28+
29+
# Smaller set used in multi-column tests for conciseness.
30+
TEST_KEYWORDS = ["pivot", "qualify", "lambda", "show"]
31+
32+
33+
def test_copy_to_parquet_with_reserved_keyword_columns(pg_conn, s3):
34+
"""
35+
COPY ... TO parquet must succeed when the table has DuckDB-reserved
36+
keyword column names, and COPY ... FROM must read the file back correctly.
37+
"""
38+
url = f"s3://{TEST_BUCKET}/test_kw_copy_parquet/kw_cols.parquet"
39+
40+
try:
41+
run_command(
42+
"""
43+
CREATE TABLE test_kw_copy_src (
44+
pivot int,
45+
qualify int,
46+
lambda int,
47+
show int
48+
);
49+
INSERT INTO test_kw_copy_src VALUES (1, 2, 3, 4), (10, 20, 30, 40);
50+
""",
51+
pg_conn,
52+
)
53+
54+
run_command(
55+
f"COPY test_kw_copy_src TO '{url}' WITH (format 'parquet')",
56+
pg_conn,
57+
)
58+
59+
run_command(
60+
f"""
61+
CREATE TABLE test_kw_copy_dst (
62+
pivot int,
63+
qualify int,
64+
lambda int,
65+
show int
66+
);
67+
COPY test_kw_copy_dst FROM '{url}' WITH (format 'parquet');
68+
""",
69+
pg_conn,
70+
)
71+
72+
result = run_query(
73+
"SELECT pivot, qualify, lambda, show FROM test_kw_copy_dst ORDER BY pivot",
74+
pg_conn,
75+
)
76+
assert result == [[1, 2, 3, 4], [10, 20, 30, 40]]
77+
finally:
78+
pg_conn.rollback()
79+
80+
81+
def test_copy_to_csv_with_reserved_keyword_columns(pg_conn, s3):
82+
"""
83+
COPY ... TO CSV and back must work with DuckDB-reserved keyword columns.
84+
"""
85+
url = f"s3://{TEST_BUCKET}/test_kw_copy_csv/kw_cols.csv"
86+
87+
try:
88+
run_command(
89+
f"""
90+
CREATE TABLE test_kw_csv_src (pivot int, qualify int, lambda int, show int);
91+
INSERT INTO test_kw_csv_src VALUES (3, 6, 9, 12), (30, 60, 90, 120);
92+
COPY test_kw_csv_src TO '{url}' WITH (format 'csv', header on);
93+
94+
CREATE TABLE test_kw_csv_dst (pivot int, qualify int, lambda int, show int);
95+
COPY test_kw_csv_dst FROM '{url}' WITH (format 'csv', header on);
96+
""",
97+
pg_conn,
98+
)
99+
100+
result = run_query(
101+
"SELECT pivot, qualify, lambda, show FROM test_kw_csv_dst ORDER BY pivot",
102+
pg_conn,
103+
)
104+
assert result == [[3, 6, 9, 12], [30, 60, 90, 120]]
105+
finally:
106+
pg_conn.rollback()
107+
108+
109+
def test_copy_to_json_with_reserved_keyword_columns(pg_conn, s3):
110+
"""
111+
COPY ... TO JSON and back must work with DuckDB-reserved keyword columns.
112+
"""
113+
url = f"s3://{TEST_BUCKET}/test_kw_copy_json/kw_cols.json"
114+
115+
try:
116+
run_command(
117+
f"""
118+
CREATE TABLE test_kw_json_src (pivot int, qualify int, lambda int, show int);
119+
INSERT INTO test_kw_json_src VALUES (7, 14, 21, 28), (70, 140, 210, 280);
120+
COPY test_kw_json_src TO '{url}' WITH (format 'json');
121+
122+
CREATE TABLE test_kw_json_dst (pivot int, qualify int, lambda int, show int);
123+
COPY test_kw_json_dst FROM '{url}' WITH (format 'json');
124+
""",
125+
pg_conn,
126+
)
127+
128+
result = run_query(
129+
"SELECT pivot, qualify, lambda, show FROM test_kw_json_dst ORDER BY pivot",
130+
pg_conn,
131+
)
132+
assert result == [[7, 14, 21, 28], [70, 140, 210, 280]]
133+
finally:
134+
pg_conn.rollback()
135+
136+
137+
# ---------------------------------------------------------------------------
138+
# Parameterised round-trip tests — each keyword, each format
139+
# ---------------------------------------------------------------------------
140+
141+
142+
@pytest.mark.parametrize("keyword", DUCKDB_ONLY_RESERVED)
143+
def test_copy_roundtrip_parquet(pg_conn, s3, keyword):
144+
"""Each DuckDB-only reserved keyword survives a parquet round-trip."""
145+
url = f"s3://{TEST_BUCKET}/test_kw_roundtrip_parquet/{keyword}/data.parquet"
146+
147+
try:
148+
run_command(
149+
f"""
150+
CREATE TABLE test_kw_pq_src_{keyword} ({keyword} int);
151+
INSERT INTO test_kw_pq_src_{keyword} VALUES (42);
152+
COPY test_kw_pq_src_{keyword} TO '{url}' WITH (format 'parquet');
153+
154+
CREATE TABLE test_kw_pq_dst_{keyword} ({keyword} int);
155+
COPY test_kw_pq_dst_{keyword} FROM '{url}' WITH (format 'parquet');
156+
""",
157+
pg_conn,
158+
)
159+
160+
result = run_query(f"SELECT {keyword} FROM test_kw_pq_dst_{keyword}", pg_conn)
161+
assert result == [[42]]
162+
finally:
163+
pg_conn.rollback()
164+
165+
166+
@pytest.mark.parametrize("keyword", DUCKDB_ONLY_RESERVED)
167+
def test_copy_roundtrip_csv(pg_conn, s3, keyword):
168+
"""Each DuckDB-only reserved keyword survives a CSV round-trip."""
169+
url = f"s3://{TEST_BUCKET}/test_kw_roundtrip_csv/{keyword}/data.csv"
170+
171+
try:
172+
run_command(
173+
f"""
174+
CREATE TABLE test_kw_csv_src_{keyword} ({keyword} int);
175+
INSERT INTO test_kw_csv_src_{keyword} VALUES (42);
176+
COPY test_kw_csv_src_{keyword} TO '{url}' WITH (format 'csv', header on);
177+
178+
CREATE TABLE test_kw_csv_dst_{keyword} ({keyword} int);
179+
COPY test_kw_csv_dst_{keyword} FROM '{url}' WITH (format 'csv', header on);
180+
""",
181+
pg_conn,
182+
)
183+
184+
result = run_query(f"SELECT {keyword} FROM test_kw_csv_dst_{keyword}", pg_conn)
185+
assert result == [[42]]
186+
finally:
187+
pg_conn.rollback()
188+
189+
190+
@pytest.mark.parametrize("keyword", DUCKDB_ONLY_RESERVED)
191+
def test_copy_roundtrip_json(pg_conn, s3, keyword):
192+
"""Each DuckDB-only reserved keyword survives a JSON round-trip."""
193+
url = f"s3://{TEST_BUCKET}/test_kw_roundtrip_json/{keyword}/data.json"
194+
195+
try:
196+
run_command(
197+
f"""
198+
CREATE TABLE test_kw_json_src_{keyword} ({keyword} int);
199+
INSERT INTO test_kw_json_src_{keyword} VALUES (42);
200+
COPY test_kw_json_src_{keyword} TO '{url}' WITH (format 'json');
201+
202+
CREATE TABLE test_kw_json_dst_{keyword} ({keyword} int);
203+
COPY test_kw_json_dst_{keyword} FROM '{url}' WITH (format 'json');
204+
""",
205+
pg_conn,
206+
)
207+
208+
result = run_query(f"SELECT {keyword} FROM test_kw_json_dst_{keyword}", pg_conn)
209+
assert result == [[42]]
210+
finally:
211+
pg_conn.rollback()

pg_lake_engine/include/pg_lake/pgduck/keywords.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,19 @@
1717

1818
#pragma once
1919

20+
/*
21+
* IsDuckDBReservedWord — returns true for any keyword that is not
22+
* UNRESERVED_KEYWORD in DuckDB (i.e., RESERVED, COL_NAME, or
23+
* TYPE_FUNC_NAME). Used for struct field-access quoting.
24+
*/
2025
PGDLLEXPORT bool IsDuckDBReservedWord(char *candidateWord);
26+
27+
/*
28+
* duckdb_quote_identifier — like quote_identifier() but also quotes
29+
* identifiers that are RESERVED_KEYWORD in DuckDB but not in PostgreSQL
30+
* (e.g. LAMBDA, PIVOT, QUALIFY, SUMMARIZE, DESCRIBE, SHOW, UNPIVOT).
31+
*
32+
* Use this for all identifiers (column names, field names, relation names)
33+
* that will appear in SQL sent to pgduck_server.
34+
*/
35+
PGDLLEXPORT const char *duckdb_quote_identifier(const char *ident);

0 commit comments

Comments
 (0)