Fix invalid SQL from index TABLESPACE clause on partitioned tables - #10342
Fix invalid SQL from index TABLESPACE clause on partitioned tables#10342dpage wants to merge 1 commit into
Conversation
PostgreSQL rejects an explicit TABLESPACE clause naming the database's
own default tablespace when the index is on a partitioned table
('cannot specify default tablespace for partitioned relations'), so
the SQL pgAdmin generated for a new index could not be executed
against a partitioned table whenever no non-default tablespace was
chosen. The index_constraint and exclusion_constraint create.sql
templates already guard against emitting this redundant clause;
extend the same guard to the plain index create.sql templates.
Closes pgadmin-org#10341
|
Warning Review limit reachedNext included review available in 52 minutes. View limit detailsLimit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (35)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The bug is real, but I think this fix makes things worse in one common configuration, so please don't merge it yet. The core problem:
Two consequences, both bad: 1. Silent wrong tablespace placement ( 2. In that same setup, the original bug isn't fixed. Both fall out of the same fix: compare against the database's actual default tablespace rather than the literal string. Also still broken: partitioned tables ( The suppression isn't limited to partitioned relations. It strips the clause from ordinary indexes' reverse-engineered SQL too. There's no partitioned indicator in the index template context today (only Cosmetic regression baked into the fixtures (32 files). Removing the clause leaves a stray blank line between The new test doesn't test what it looks like it tests ( No injection concern — |
2 participants
Summary
TABLESPACEclause naming the database's own default tablespace when the index belongs to a partitioned table (cannot specify default tablespace for partitioned relations), so the SQL pgAdmin generated for a new index on a partitioned table could not be executed whenever the Tablespace field was left at its default (pg_default).index_constraintandexclusion_constraintcreate.sql templates already guard against emitting this redundant clause (data.spcname != "pg_default"); this extends the same guard to the plain indexcreate.sqltemplates (defaultand15_plusbuckets).indexes/tests/{default,15_plus}to match the corrected output.Test plan
test_create_sql_partitioned_tablespace.py, a DB-free unit test rendering the templates directly and asserting the redundant clause is omitted while an explicit non-default tablespace is still emitted.regression/runtests.py --pkg browser.server_groups.servers.databases.schemas.tables.indexes.tests— 49/49 passing.regression/runtests.py --pkg browser.server_groups.servers.databases.schemas.tables.tests— 113/113 passing (no regression in the parent Tables SQL, which embeds indexes).CREATE INDEX ... TABLESPACE pg_defaulton a partitioned table errors, while a real non-default tablespace works fine.Closes #10341