Fix invalid SQL from index TABLESPACE clause on partitioned tables - #10342
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
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (35)
WalkthroughThe index SQL templates now omit ChangesIndex tablespace SQL generation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
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 — |
hiteshjambhale
left a comment
hiteshjambhale
left a comment
There was a problem hiding this comment.
Tested manually on master and PR branch.
Master: Creating an index on a partitioned table with pg_default tablespace generates TABLESPACE pg_default in the SQL — Postgres rejects it with "cannot specify default tablespace for partitioned relations".
PR branch: TABLESPACE pg_default is correctly omitted — index creates successfully. Custom tablespaces (e.g. eav) still appear in the SQL as expected.
3 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
Summary by CodeRabbit
Bug Fixes
pg_default, allowing partitioned indexes to be created successfully.Tests