Skip to content

Backport #8872: Quote the tablespace name in REINDEX commands sent to shards (release-13.2) - #8911

Open
ibrahim halatci (ihalatci) wants to merge 1 commit into
bp-8873-release-13.2from
bp-8872-release-13.2
Open

ibrahim halatci (ihalatci) wants to merge 1 commit into
bp-8873-release-13.2from
bp-8872-release-13.2

Conversation

@ihalatci

Copy link
Copy Markdown
Contributor

Backports #8872 (commit 76a8377) to release-13.2.

Description

Quote the tablespace name in \REINDEX (TABLESPACE ...)\ commands sent to shards using \quote_identifier, preventing syntax errors or unintended lowercasing on workers.

Validation

Tested locally on PostgreSQL 17 with regression test \pg14.

@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 22.71%. Comparing base (6d8ea44) to head (f6e5c24).

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (22.71%) is below the target coverage (87.50%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (6d8ea44) and HEAD (f6e5c24). Click for more details.

HEAD has 86 uploads less than BASE
Flag BASE (6d8ea44) HEAD (f6e5c24)
15_citus_upgrade 1 0
16_regress_check-pytest 1 0
17_regress_check-pytest 1 0
15_regress_check-pytest 1 0
17_regress_check-follower-cluster_v13.4.0_all 1 0
16_17_upgrade 1 0
17_regress_check-follower-cluster_13.4-1_all 1 0
17_regress_check-follower-cluster_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-columnar-isolation_v13.4.0_all 1 0
17_regress_check-columnar-isolation_13.4-1_v13.4.0_coordinatoronly 1 0
17_regress_check-enterprise-isolation-logicalrep-2_13.4-1_v13.4.0_coordinatoronly 1 0
15_regress_check-follower-cluster 1 0
17_regress_check-columnar-isolation 1 0
17_regress_check-columnar_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-columnar-isolation_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-add-backup-node_v13.4.0_all 1 0
16_regress_check-follower-cluster 1 0
17_regress_check-add-backup-node_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-split_13.4-1_v13.4.0_coordinatoronly 1 0
15_regress_check-columnar-isolation 1 0
15_arbitrary_configs_3 1 0
17_regress_check-enterprise-isolation-logicalrep-2 1 0
16_regress_check-enterprise-failure 1 0
16_regress_check-enterprise-isolation-logicalrep-3 1 0
15_regress_check-add-backup-node 1 0
17_regress_check-enterprise-isolation_13.4-1_v13.4.0_coordinatoronly 1 0
15_16_upgrade 1 0
17_regress_check-add-backup-node_13.4-1_all 1 0
15_regress_check-split-output-plugin-denied 1 0
15_regress_check-enterprise-isolation-logicalrep-3 1 0
16_regress_check-enterprise-isolation 1 0
17_regress_check-enterprise_13.4-1_v13.4.0_coordinatoronly 1 0
15_regress_check-vanilla 1 0
17_regress_check-enterprise-isolation-logicalrep-1 1 0
17_regress_check-enterprise-isolation-logicalrep-3_13.4-1_all 1 0
17_regress_check-columnar_v13.4.0_all 1 0
17_arbitrary_configs_4 1 0
15_arbitrary_configs_4 1 0
16_arbitrary_configs_4 1 0
16_regress_check-columnar 1 0
17_regress_check-enterprise_v13.4.0_all 1 0
17_regress_check-split_13.4-1_all 1 0
17_regress_check-enterprise-isolation_v13.4.0_all 1 0
16_regress_check-multi-mx 1 0
15_regress_check-operations 1 0
17_regress_check-operations 1 0
17_regress_check-operations_13.4-1_all 1 0
17_regress_check-operations_v13.4.0_all 1 0
17_regress_check-enterprise-isolation-logicalrep-1_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-split_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-columnar_13.4-1_v13.4.0_coordinatoronly 1 0
15_regress_check-enterprise 1 0
15_regress_check-split 1 0
17_cdc_installcheck 1 0
17_regress_check-split 1 0
17_regress_check-enterprise-isolation_13.4-1_all 1 0
17_regress_check-failure 1 0
16_regress_check-failure 1 0
15_regress_check-failure 1 0
16_regress_check-vanilla 1 0
17_regress_check-vanilla 1 0
16_regress_check-split 1 0
17_regress_check-enterprise-isolation-logicalrep-1_13.4-1_v13.4.0_coordinatoronly 1 0
16_regress_check-enterprise-isolation-logicalrep-1 1 0
16_regress_check-multi-1-create-citus 1 0
15_regress_check-multi-1 1 0
16_regress_check-multi-1 1 0
17_regress_check-multi-1_13.4-1_v13.4.0_coordinatoronly 1 0
17_regress_check-operations_13.4-1_v13.4.0_coordinatoronly 1 0
17_regress_check-isolation_13.4-1_v13.4.0_coordinatoronly 1 0
17_regress_check-isolation_13.4-1_v13.4.0_workeronly 1 0
17_regress_check-isolation_v13.4.0_all 1 0
15_arbitrary_configs_2 1 0
17_regress_check-multi-1_v13.4.0_all 1 0
16_regress_check-multi 1 0
17_regress_check-enterprise-isolation-logicalrep-2_13.4-1_all 1 0
17_regress_check-multi_13.4-1_all 1 0
17_regress_check-enterprise_13.4-1_v13.4.0_workeronly 1 0
15_regress_check-multi 1 0
15_regress_check-enterprise-isolation-logicalrep-1 1 0
16_arbitrary_configs_5 1 0
17_regress_check-multi-1-create-citus 1 0
17_regress_check-follower-cluster_13.4-1_v13.4.0_coordinatoronly 1 0
17_regress_check-enterprise-isolation-logicalrep-3 1 0
15_regress_check-enterprise-isolation-logicalrep-2 1 0
17_arbitrary_configs_1 1 0
Additional details and impacted files
@@                Coverage Diff                @@
##           release-13.2    #8911       +/-   ##
=================================================
- Coverage         88.69%   22.71%   -65.98%     
=================================================
  Files               289      289               
  Lines             64641    63411     -1230     
  Branches           8153     7877      -276     
=================================================
- Hits              57333    14405    -42928     
- Misses             4889    47174    +42285     
+ Partials           2419     1832      -587     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

DESCRIPTION: Fixes REINDEX with a tablespace name that needs quoting

`REINDEX (TABLESPACE "Test Tablespace") INDEX idx` on a distributed table fails, because the per-shard command is built with the tablespace name unquoted (`AddVacuumParams` in `citus_ruleutils.c`):

```
reindex(TABLESPACE "Test Tablespace") index idx;
ERROR:  syntax error at or near "Tablespace"
CONTEXT:  while executing command on localhost:57637
```

A mixed-case name like `"TS1"` is downcased on the workers instead, so it points at a different tablespace or at one that does not exist. The fix quotes the name with `quote_identifier`, the same way the rest of the statement is deparsed.

**Test.** I added a case to `pg14.sql` next to the existing `reindex(TABLESPACE ...)` tests. It renames `test_tablespace` to `"Test Tablespace"` on all nodes, reindexes, and renames it back.

- On `main` @ c267b0b, the test fails with the syntax error above.
- With the fix, it passes.

I ran this on PG 17.11 in Docker with `make check-custom-schedule` (minimal cluster setup + `tablespace` + `pg14`). One local-only change was needed: `pg14` has `citus_move_shard_placement` cases that failed on my host's disk-space check, so I set `citus.desired_percent_disk_available_after_move=0` in `pg_regress_multi.pl`. That change is not part of this PR. `uncrustify` with the citus-style config reports no changes.

This PR was prepared with an AI coding agent (Claude) working for the breken-ai account. I reviewed the change and the test output.

(cherry picked from commit 76a8377)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d89d1f43-18ea-4405-bd1b-5c3129e391a9
@ihalatci
ibrahim halatci (ihalatci) changed the base branch from release-13.2 to bp-8873-release-13.2 October 14, 2026 07:35
@ihalatci
ibrahim halatci (ihalatci) added this pull request to stack #8928 October 14, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants