From b10f2e0ee03476efd9823b70f14e7fb8ba85d12a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= Date: Sat, 11 Apr 2026 03:51:22 +0200 Subject: [PATCH 1/2] Fail with a clear error when scanning a partitioned table Scanning a partitioned table caused an infinite hang because PostgreSQL sets relpages=-1 for relkind='p' tables. Cast to idx_t (uint64) gives pages_approx = 2^64-1, so the scanner loops over ~18 quintillion pages. Now throws NotImplementedException at bind time with a message that includes the relation kind and table name. --- src/include/postgres_utils.hpp | 2 ++ src/postgres_utils.cpp | 15 +++++++++++++++ src/storage/postgres_table_set.cpp | 25 +++++++++++++++++++------ test/other.sql | 7 +++++++ test/sql/scanner/partitioned_table.test | 20 ++++++++++++++++++++ 5 files changed, 63 insertions(+), 6 deletions(-) create mode 100644 test/sql/scanner/partitioned_table.test diff --git a/src/include/postgres_utils.hpp b/src/include/postgres_utils.hpp index 40f151698..5c8ed35ae 100644 --- a/src/include/postgres_utils.hpp +++ b/src/include/postgres_utils.hpp @@ -72,6 +72,8 @@ class PostgresUtils { static PostgresType CreateEmptyPostgresType(const LogicalType &type); static string QuotePostgresIdentifier(const string &text); + static string RelkindToString(const string &relkind); + static PostgresVersion ExtractPostgresVersion(const string &version); }; diff --git a/src/postgres_utils.cpp b/src/postgres_utils.cpp index 646696b62..2cfe6190a 100644 --- a/src/postgres_utils.cpp +++ b/src/postgres_utils.cpp @@ -481,6 +481,21 @@ PostgresVersion PostgresUtils::ExtractPostgresVersion(const string &version_str) return result; } +string PostgresUtils::RelkindToString(const string &relkind) { + if (relkind == "r") { + return "table"; + } else if (relkind == "v") { + return "view"; + } else if (relkind == "m") { + return "materialized view"; + } else if (relkind == "f") { + return "foreign table"; + } else if (relkind == "p") { + return "partitioned table"; + } + return "relation (relkind=" + relkind + ")"; +} + string PostgresUtils::QuotePostgresIdentifier(const string &text) { return KeywordHelper::WriteOptionallyQuoted(text, '"', false); } diff --git a/src/storage/postgres_table_set.cpp b/src/storage/postgres_table_set.cpp index 321d62e22..c34381b7d 100644 --- a/src/storage/postgres_table_set.cpp +++ b/src/storage/postgres_table_set.cpp @@ -24,7 +24,7 @@ string PostgresTableSet::GetInitializeQuery(const string &schema, const string & SELECT pg_namespace.oid AS namespace_id, relname, relpages, attname, pg_type.typname type_name, atttypmod type_modifier, pg_attribute.attndims ndim, attnum, pg_attribute.attnotnull AS notnull, NULL constraint_id, - NULL constraint_type, NULL constraint_key + NULL constraint_type, NULL constraint_key, pg_class.relkind FROM pg_class JOIN pg_namespace ON relnamespace = pg_namespace.oid JOIN pg_attribute ON pg_class.oid=pg_attribute.attrelid @@ -34,7 +34,7 @@ UNION ALL SELECT pg_namespace.oid AS namespace_id, relname, NULL relpages, NULL attname, NULL type_name, NULL type_modifier, NULL ndim, NULL attnum, NULL AS notnull, pg_constraint.oid AS constraint_id, contype AS constraint_type, - conkey AS constraint_key + conkey AS constraint_key, NULL AS relkind FROM pg_class JOIN pg_namespace ON relnamespace = pg_namespace.oid JOIN pg_constraint ON (pg_class.oid=pg_constraint.conrelid) @@ -51,6 +51,14 @@ ORDER BY namespace_id, relname, attnum, constraint_id; return StringUtil::Replace(base_query, "${CONDITION}", condition); } +static idx_t GetScanRelpages(int64_t relpages, const string &relkind, const string &table_name) { + if (relpages < 0) { + throw NotImplementedException("Scanning %s \"%s\" is not supported (pg_class returned negative relpages).", + PostgresUtils::RelkindToString(relkind), table_name); + } + return static_cast(relpages); +} + void PostgresTableSet::AddColumn(optional_ptr transaction, optional_ptr schema, PostgresResult &result, idx_t row, PostgresTableInfo &table_info) { @@ -128,9 +136,10 @@ void PostgresTableSet::CreateEntries(PostgresTransaction &transaction, PostgresR if (info) { tables.push_back(std::move(info)); } - auto approx_num_pages = result.IsNull(row, 2) ? 0 : result.GetInt64(row, 2); info = make_uniq(schema, table_name); - info->approx_num_pages = approx_num_pages; + auto relpages = result.IsNull(row, 2) ? 0 : result.GetInt64(row, 2); + auto relkind = result.IsNull(row, 12) ? string() : result.GetString(row, 12); + info->approx_num_pages = GetScanRelpages(relpages, relkind, table_name); } AddColumnOrConstraint(&transaction, &schema, result, row, *info); } @@ -169,7 +178,9 @@ unique_ptr PostgresTableSet::GetTableInfo(PostgresTransaction for (idx_t row = 0; row < rows; row++) { AddColumnOrConstraint(&transaction, &schema, *result, row, *table_info); } - table_info->approx_num_pages = result->GetInt64(0, 2); + auto relpages = result->IsNull(0, 2) ? 0 : result->GetInt64(0, 2); + auto relkind = result->IsNull(0, 12) ? string() : result->GetString(0, 12); + table_info->approx_num_pages = GetScanRelpages(relpages, relkind, table_name); return table_info; } @@ -185,7 +196,9 @@ unique_ptr PostgresTableSet::GetTableInfo(ClientContext &cont for (idx_t row = 0; row < rows; row++) { AddColumnOrConstraint(nullptr, nullptr, *result, row, *table_info); } - table_info->approx_num_pages = result->GetInt64(0, 2); + auto relpages = result->IsNull(0, 2) ? 0 : result->GetInt64(0, 2); + auto relkind = result->IsNull(0, 12) ? string() : result->GetString(0, 12); + table_info->approx_num_pages = GetScanRelpages(relpages, relkind, table_name); return table_info; } diff --git a/test/other.sql b/test/other.sql index 5406c854a..d25f5e1aa 100644 --- a/test/other.sql +++ b/test/other.sql @@ -168,3 +168,10 @@ create table tbl_with_unique_constraints(pk int unique, c1 int not null, c2 int, create schema main; create table main.main_tbl(i int); insert into main.main_tbl values (42), (NULL); + +-- partitioned table (parent has relpages=-1 after analyze) +create table part_tbl (id int, val text) partition by range (id); +create table part_tbl_1 partition of part_tbl for values from (1) to (100); +create table part_tbl_2 partition of part_tbl for values from (100) to (200); +insert into part_tbl values (1,'a'),(50,'b'),(100,'c'),(150,'d'); +analyze part_tbl; diff --git a/test/sql/scanner/partitioned_table.test b/test/sql/scanner/partitioned_table.test new file mode 100644 index 000000000..c7481e4bd --- /dev/null +++ b/test/sql/scanner/partitioned_table.test @@ -0,0 +1,20 @@ +# name: test/sql/scanner/partitioned_table.test +# description: Scanning a partitioned table gracefully fail +# group: [scanner] + +require postgres_scanner + +require-env POSTGRES_TEST_DATABASE_AVAILABLE + +statement ok +ATTACH 'dbname=postgresscanner' AS s (TYPE POSTGRES) + +statement error +SELECT * FROM s.public.part_tbl +---- +Scanning partitioned table "part_tbl" is not supported (pg_class returned negative relpages). + +statement error +EXPLAIN SELECT * FROM s.public.part_tbl +---- +Scanning partitioned table "part_tbl" is not supported (pg_class returned negative relpages). From 808632d7c3aec9f57839da469453fab4eeb26e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= Date: Sun, 12 Apr 2026 13:48:39 +0200 Subject: [PATCH 2/2] Fallback to 0 approx_num_rows for -1 relpages. - fixes hanging partitioned table scan for now --- src/include/postgres_scanner.hpp | 2 +- src/include/postgres_utils.hpp | 2 -- src/include/storage/postgres_table_entry.hpp | 4 ++-- src/postgres_scanner.cpp | 8 +++++-- src/postgres_utils.cpp | 15 ------------ src/storage/postgres_table_set.cpp | 24 ++++---------------- test/sql/scanner/partitioned_table.test | 15 +++++++----- 7 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/include/postgres_scanner.hpp b/src/include/postgres_scanner.hpp index 3e0162cdc..3bf4bd5ad 100644 --- a/src/include/postgres_scanner.hpp +++ b/src/include/postgres_scanner.hpp @@ -84,7 +84,7 @@ class PostgresScanFunction : public TableFunction { PostgresScanFunction(); static void PrepareBind(PostgresVersion version, ClientContext &context, PostgresBindData &bind, - idx_t approx_num_pages); + int64_t approx_num_pages); }; class PostgresScanFunctionFilterPushdown : public TableFunction { diff --git a/src/include/postgres_utils.hpp b/src/include/postgres_utils.hpp index 5c8ed35ae..40f151698 100644 --- a/src/include/postgres_utils.hpp +++ b/src/include/postgres_utils.hpp @@ -72,8 +72,6 @@ class PostgresUtils { static PostgresType CreateEmptyPostgresType(const LogicalType &type); static string QuotePostgresIdentifier(const string &text); - static string RelkindToString(const string &relkind); - static PostgresVersion ExtractPostgresVersion(const string &version); }; diff --git a/src/include/storage/postgres_table_entry.hpp b/src/include/storage/postgres_table_entry.hpp index 529c23408..02da22ff9 100644 --- a/src/include/storage/postgres_table_entry.hpp +++ b/src/include/storage/postgres_table_entry.hpp @@ -35,7 +35,7 @@ struct PostgresTableInfo { unique_ptr create_info; vector postgres_types; vector postgres_names; - idx_t approx_num_pages = 0; + int64_t approx_num_pages = 0; }; class PostgresTableEntry : public TableCatalogEntry { @@ -64,7 +64,7 @@ class PostgresTableEntry : public TableCatalogEntry { //! We would in this case remap them to "ID" and "id:1", while postgres_names store the original names vector postgres_names; //! The approximate number of pages a table consumes in Postgres - idx_t approx_num_pages; + int64_t approx_num_pages; }; } // namespace duckdb diff --git a/src/postgres_scanner.cpp b/src/postgres_scanner.cpp index ba0cd6e79..73023cfeb 100644 --- a/src/postgres_scanner.cpp +++ b/src/postgres_scanner.cpp @@ -107,7 +107,7 @@ static void PostgresGetSnapshot(ClientContext &context, PostgresVersion version, } void PostgresScanFunction::PrepareBind(PostgresVersion version, ClientContext &context, PostgresBindData &bind_data, - idx_t approx_num_pages) { + int64_t approx_num_pages) { Value pages_per_task; if (context.TryGetCurrentSetting("pg_pages_per_task", pages_per_task)) { bind_data.pages_per_task = UBigIntValue::Get(pages_per_task); @@ -130,10 +130,14 @@ void PostgresScanFunction::PrepareBind(PostgresVersion version, ClientContext &c // see https://github.com/duckdb/postgres_scanner/issues/186 use_ctid_scan = false; } + if (approx_num_pages < 0) { + // negative relpages (e.g. partitioned tables) cannot use ctid scan + use_ctid_scan = false; + } if (!use_ctid_scan) { approx_num_pages = 0; } - bind_data.SetTablePages(approx_num_pages); + bind_data.SetTablePages(static_cast(approx_num_pages)); bind_data.version = version; if (version.type_v == PostgresInstanceType::REDSHIFT) { bind_data.use_text_protocol = true; diff --git a/src/postgres_utils.cpp b/src/postgres_utils.cpp index 2cfe6190a..646696b62 100644 --- a/src/postgres_utils.cpp +++ b/src/postgres_utils.cpp @@ -481,21 +481,6 @@ PostgresVersion PostgresUtils::ExtractPostgresVersion(const string &version_str) return result; } -string PostgresUtils::RelkindToString(const string &relkind) { - if (relkind == "r") { - return "table"; - } else if (relkind == "v") { - return "view"; - } else if (relkind == "m") { - return "materialized view"; - } else if (relkind == "f") { - return "foreign table"; - } else if (relkind == "p") { - return "partitioned table"; - } - return "relation (relkind=" + relkind + ")"; -} - string PostgresUtils::QuotePostgresIdentifier(const string &text) { return KeywordHelper::WriteOptionallyQuoted(text, '"', false); } diff --git a/src/storage/postgres_table_set.cpp b/src/storage/postgres_table_set.cpp index c34381b7d..8df2c7b82 100644 --- a/src/storage/postgres_table_set.cpp +++ b/src/storage/postgres_table_set.cpp @@ -24,7 +24,7 @@ string PostgresTableSet::GetInitializeQuery(const string &schema, const string & SELECT pg_namespace.oid AS namespace_id, relname, relpages, attname, pg_type.typname type_name, atttypmod type_modifier, pg_attribute.attndims ndim, attnum, pg_attribute.attnotnull AS notnull, NULL constraint_id, - NULL constraint_type, NULL constraint_key, pg_class.relkind + NULL constraint_type, NULL constraint_key FROM pg_class JOIN pg_namespace ON relnamespace = pg_namespace.oid JOIN pg_attribute ON pg_class.oid=pg_attribute.attrelid @@ -34,7 +34,7 @@ UNION ALL SELECT pg_namespace.oid AS namespace_id, relname, NULL relpages, NULL attname, NULL type_name, NULL type_modifier, NULL ndim, NULL attnum, NULL AS notnull, pg_constraint.oid AS constraint_id, contype AS constraint_type, - conkey AS constraint_key, NULL AS relkind + conkey AS constraint_key FROM pg_class JOIN pg_namespace ON relnamespace = pg_namespace.oid JOIN pg_constraint ON (pg_class.oid=pg_constraint.conrelid) @@ -51,14 +51,6 @@ ORDER BY namespace_id, relname, attnum, constraint_id; return StringUtil::Replace(base_query, "${CONDITION}", condition); } -static idx_t GetScanRelpages(int64_t relpages, const string &relkind, const string &table_name) { - if (relpages < 0) { - throw NotImplementedException("Scanning %s \"%s\" is not supported (pg_class returned negative relpages).", - PostgresUtils::RelkindToString(relkind), table_name); - } - return static_cast(relpages); -} - void PostgresTableSet::AddColumn(optional_ptr transaction, optional_ptr schema, PostgresResult &result, idx_t row, PostgresTableInfo &table_info) { @@ -137,9 +129,7 @@ void PostgresTableSet::CreateEntries(PostgresTransaction &transaction, PostgresR tables.push_back(std::move(info)); } info = make_uniq(schema, table_name); - auto relpages = result.IsNull(row, 2) ? 0 : result.GetInt64(row, 2); - auto relkind = result.IsNull(row, 12) ? string() : result.GetString(row, 12); - info->approx_num_pages = GetScanRelpages(relpages, relkind, table_name); + info->approx_num_pages = result.IsNull(row, 2) ? 0 : result.GetInt64(row, 2); } AddColumnOrConstraint(&transaction, &schema, result, row, *info); } @@ -178,9 +168,7 @@ unique_ptr PostgresTableSet::GetTableInfo(PostgresTransaction for (idx_t row = 0; row < rows; row++) { AddColumnOrConstraint(&transaction, &schema, *result, row, *table_info); } - auto relpages = result->IsNull(0, 2) ? 0 : result->GetInt64(0, 2); - auto relkind = result->IsNull(0, 12) ? string() : result->GetString(0, 12); - table_info->approx_num_pages = GetScanRelpages(relpages, relkind, table_name); + table_info->approx_num_pages = result->IsNull(0, 2) ? 0 : result->GetInt64(0, 2); return table_info; } @@ -196,9 +184,7 @@ unique_ptr PostgresTableSet::GetTableInfo(ClientContext &cont for (idx_t row = 0; row < rows; row++) { AddColumnOrConstraint(nullptr, nullptr, *result, row, *table_info); } - auto relpages = result->IsNull(0, 2) ? 0 : result->GetInt64(0, 2); - auto relkind = result->IsNull(0, 12) ? string() : result->GetString(0, 12); - table_info->approx_num_pages = GetScanRelpages(relpages, relkind, table_name); + table_info->approx_num_pages = result->IsNull(0, 2) ? 0 : result->GetInt64(0, 2); return table_info; } diff --git a/test/sql/scanner/partitioned_table.test b/test/sql/scanner/partitioned_table.test index c7481e4bd..f25b56860 100644 --- a/test/sql/scanner/partitioned_table.test +++ b/test/sql/scanner/partitioned_table.test @@ -1,5 +1,5 @@ # name: test/sql/scanner/partitioned_table.test -# description: Scanning a partitioned table gracefully fail +# description: Scanning a partitioned table fallbacks to 0 estimate # group: [scanner] require postgres_scanner @@ -9,12 +9,15 @@ require-env POSTGRES_TEST_DATABASE_AVAILABLE statement ok ATTACH 'dbname=postgresscanner' AS s (TYPE POSTGRES) -statement error -SELECT * FROM s.public.part_tbl +query IT +SELECT * FROM s.public.part_tbl ORDER BY id ---- -Scanning partitioned table "part_tbl" is not supported (pg_class returned negative relpages). +1 a +50 b +100 c +150 d -statement error +query II EXPLAIN SELECT * FROM s.public.part_tbl ---- -Scanning partitioned table "part_tbl" is not supported (pg_class returned negative relpages). +physical_plan :.*~0 rows.*