From e54875775a01f0d366840790008f8bd803b593ce Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Tue, 28 Jan 2025 18:40:05 +0100 Subject: [PATCH] Fix #284: quote uppercase identifiers when shipping queries to Postgres --- src/include/postgres_utils.hpp | 1 + src/postgres_utils.cpp | 4 +++ src/storage/postgres_delete.cpp | 2 +- src/storage/postgres_index_set.cpp | 6 ++-- src/storage/postgres_schema_entry.cpp | 6 ++-- src/storage/postgres_update.cpp | 4 +-- test/sql/storage/attach_upper_case.test | 39 +++++++++++++++++++++++++ 7 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 test/sql/storage/attach_upper_case.test diff --git a/src/include/postgres_utils.hpp b/src/include/postgres_utils.hpp index ebeb37fa..34e5ee5c 100644 --- a/src/include/postgres_utils.hpp +++ b/src/include/postgres_utils.hpp @@ -68,6 +68,7 @@ class PostgresUtils { static bool SupportedPostgresOid(const LogicalType &input); static LogicalType RemoveAlias(const LogicalType &type); static PostgresType CreateEmptyPostgresType(const LogicalType &type); + static string QuotePostgresIdentifier(const string &text); static PostgresVersion ExtractPostgresVersion(const string &version); }; diff --git a/src/postgres_utils.cpp b/src/postgres_utils.cpp index a178af0a..64715d62 100644 --- a/src/postgres_utils.cpp +++ b/src/postgres_utils.cpp @@ -475,4 +475,8 @@ PostgresVersion PostgresUtils::ExtractPostgresVersion(const string &version_str) return result; } +string PostgresUtils::QuotePostgresIdentifier(const string &text) { + return KeywordHelper::WriteOptionallyQuoted(text, '"', false); +} + } // namespace duckdb diff --git a/src/storage/postgres_delete.cpp b/src/storage/postgres_delete.cpp index 814bd185..97c0f8c9 100644 --- a/src/storage/postgres_delete.cpp +++ b/src/storage/postgres_delete.cpp @@ -19,7 +19,7 @@ string GetDeleteSQL(const PostgresTableEntry &table, const string &ctid_list) { string result; result = "DELETE FROM "; result += KeywordHelper::WriteQuoted(table.schema.name, '"') + "."; - result += KeywordHelper::WriteOptionallyQuoted(table.name); + result += PostgresUtils::QuotePostgresIdentifier(table.name); result += " WHERE ctid IN (" + ctid_list + ")"; return result; } diff --git a/src/storage/postgres_index_set.cpp b/src/storage/postgres_index_set.cpp index 0ef12aa6..64533a63 100644 --- a/src/storage/postgres_index_set.cpp +++ b/src/storage/postgres_index_set.cpp @@ -61,10 +61,10 @@ string PGGetCreateIndexSQL(CreateIndexInfo &info, TableCatalogEntry &tbl) { sql += " UNIQUE"; } sql += " INDEX "; - sql += KeywordHelper::WriteOptionallyQuoted(info.index_name); + sql += PostgresUtils::QuotePostgresIdentifier(info.index_name); sql += " ON "; - sql += KeywordHelper::WriteOptionallyQuoted(tbl.schema.name) + "."; - sql += KeywordHelper::WriteOptionallyQuoted(tbl.name); + sql += PostgresUtils::QuotePostgresIdentifier(tbl.schema.name) + "."; + sql += PostgresUtils::QuotePostgresIdentifier(tbl.name); sql += "("; for (idx_t i = 0; i < info.parsed_expressions.size(); i++) { if (i > 0) { diff --git a/src/storage/postgres_schema_entry.cpp b/src/storage/postgres_schema_entry.cpp index e954f85b..32e2f90c 100644 --- a/src/storage/postgres_schema_entry.cpp +++ b/src/storage/postgres_schema_entry.cpp @@ -73,8 +73,8 @@ optional_ptr PostgresSchemaEntry::CreateIndex(CatalogTransaction t string PGGetCreateViewSQL(PostgresSchemaEntry &schema, CreateViewInfo &info) { string sql; sql = "CREATE VIEW "; - sql += KeywordHelper::WriteOptionallyQuoted(schema.name) + "."; - sql += KeywordHelper::WriteOptionallyQuoted(info.view_name); + sql += PostgresUtils::QuotePostgresIdentifier(schema.name) + "."; + sql += PostgresUtils::QuotePostgresIdentifier(info.view_name); sql += " "; if (!info.aliases.empty()) { sql += "("; @@ -83,7 +83,7 @@ string PGGetCreateViewSQL(PostgresSchemaEntry &schema, CreateViewInfo &info) { sql += ", "; } auto &alias = info.aliases[i]; - sql += KeywordHelper::WriteOptionallyQuoted(alias); + sql += PostgresUtils::QuotePostgresIdentifier(alias); } sql += ") "; } diff --git a/src/storage/postgres_update.cpp b/src/storage/postgres_update.cpp index 90e7312b..4f0aefa9 100644 --- a/src/storage/postgres_update.cpp +++ b/src/storage/postgres_update.cpp @@ -30,7 +30,7 @@ class PostgresUpdateGlobalState : public GlobalSinkState { string CreateUpdateTable(const string &name, PostgresTableEntry &table, const vector &index) { string result; - result = "CREATE LOCAL TEMPORARY TABLE " + KeywordHelper::WriteOptionallyQuoted(name); + result = "CREATE LOCAL TEMPORARY TABLE " + PostgresUtils::QuotePostgresIdentifier(name); result += "("; for (idx_t i = 0; i < index.size(); i++) { if (i > 0) { @@ -63,7 +63,7 @@ string GetUpdateSQL(const string &name, PostgresTableEntry &table, const vector< result += "."; result += KeywordHelper::WriteQuoted(column_name, '"'); } - result += " FROM " + KeywordHelper::WriteOptionallyQuoted(name); + result += " FROM " + PostgresUtils::QuotePostgresIdentifier(name); result += " WHERE "; result += KeywordHelper::WriteQuoted(table.name, '"'); result += ".ctid=__page_id_string::TID"; diff --git a/test/sql/storage/attach_upper_case.test b/test/sql/storage/attach_upper_case.test new file mode 100644 index 00000000..01326543 --- /dev/null +++ b/test/sql/storage/attach_upper_case.test @@ -0,0 +1,39 @@ +# name: test/sql/storage/attach_upper_case.test +# description: Test modifying an upper-case table +# group: [storage] + +require postgres_scanner + +require-env POSTGRES_TEST_DATABASE_AVAILABLE + +statement ok +PRAGMA enable_verification + +statement ok +ATTACH 'dbname=postgresscanner' AS s1 (TYPE POSTGRES) + +statement ok +USE s1 + +statement ok +DROP SCHEMA IF EXISTS SCHEM01 CASCADE + +statement ok +CREATE SCHEMA SCHEM01 + +statement ok +create or replace table SCHEM01.TAB01(COL01 VARCHAR); + +statement ok +insert into SCHEM01.TAB01 values ('abc') + +statement ok +update SCHEM01.TAB01 set COL01='zxcv' + +query I +SELECT COL01 FROM SCHEM01.TAB01 +---- +zxcv + +statement ok +truncate SCHEM01.tab01