Fix CREATE EXTENSION IF NOT EXISTS for non-owner users - #8465
Conversation
Co-authored-by: onurctirtir <16804727+onurctirtir@users.noreply.github.com>
Co-authored-by: onurctirtir <16804727+onurctirtir@users.noreply.github.com>
Co-authored-by: onurctirtir <16804727+onurctirtir@users.noreply.github.com>
|
Copilot, please try to make your changes on that test file in an idempotent way, please see the CI failures |
Co-authored-by: onurctirtir <16804727+onurctirtir@users.noreply.github.com>
Fixed in commit 4453704. The test now uses |
|
Copilot, please re-check the test failures, some parts of the test are still breaking the idempotency |
… schema drop Co-authored-by: onurctirtir <16804727+onurctirtir@users.noreply.github.com>
Fixed in commit d02b399. The remaining idempotency issues were:
The test should now be fully idempotent and handle multiple runs without errors. |
|
Copilot, let's give it one last trial :/ : https://github.com/citusdata/citus/actions/runs/21708232641?pr=8465 |
… extension and test_extension_function table operations Co-authored-by: onurctirtir <16804727+onurctirtir@users.noreply.github.com>
All idempotency issues have been addressed in commit 6b4ae61 (same fixes as mentioned in my previous response). The test now includes IF EXISTS/IF NOT EXISTS for all operations that could fail on subsequent runs:
The test should now pass all CI checks. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8465 +/- ##
=======================================
Coverage 88.92% 88.92%
=======================================
Files 286 286
Lines 63102 63103 +1
Branches 7907 7909 +2
=======================================
+ Hits 56113 56114 +1
Misses 4733 4733
Partials 2256 2256 🚀 New features to boost your workflow:
|
…#8465) (#8718) Backport of #8465 to `release-14.0` (fixes #7091). Tracking issue: #8713. **Verbatim cherry-pick** — `git cherry-pick -x fde8cea`, zero conflicts, **zero adaptation**. `git patch-id --stable` is identical to the upstream commit. ### The bug `CREATE EXTENSION IF NOT EXISTS <ext>` failed for a non-owner user even when the extension was already installed. Citus propagated the statement without preserving `if_not_exists`, so the remote side attempted a real create and rejected it: ``` ERROR: must be owner of extension seg ``` `IF NOT EXISTS` is supposed to be a no-op in exactly this situation. ### The fix Set `if_not_exists` on the propagated statement at the three sites that construct it (`extension.c`). ### Why this cannot change behavior on 14.0 The change only makes the propagated statement carry the same `IF NOT EXISTS` semantics the user actually wrote. When the extension is absent the statement still creates it; when it is present the remote side now no-ops instead of erroring. No path that succeeds today changes outcome. ### Applicability to release-14.0 Vulnerable code confirmed present before the pick: `extension.c:185,1146,1318` never set `if_not_exists`. `git diff fde8cea^ release-14.0` over the touched files was empty — the branch was in the exact pre-fix state. Note this fix is missing from **all three** maintained release lines, not just 14.0; it is included here because `release-14.0` is the active 14.x line. ### Validation Branch is 1 ahead / 0 behind `8cdb17e25`. Full A/B on PG16.14 (WSL), unmodified `release-14.0` vs a stack of all five backports, **with `make install` before each leg and an assertion that the installed `citus.so` md5 matches the worktree build**: | leg | `check-multi` | `check-multi-1` | |---|---|---| | baseline `8cdb17e25` | 190 ok, 0 failed | 207 ok, 0 failed | | all five stacked | 190 ok, 0 failed | 207 ok, 0 failed | `propagate_extension_commands` — which this PR extends — is in `multi_1_schedule` (`check-multi-1`) and passes on both legs. Normalized status sets are **byte-identical**, with zero `not ok` lines on either side. `release-14.0` has **no** `_N` numbered expected-output variants anywhere on the branch, so there is no PG-version-specific variant of the touched expected file that could go unchecked. Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Release prep for **citus v14.2.0** from `release-14.0`. DESCRIPTION: add changelog for 14.2.0 All version-bearing files (configure.ac, configure, citus.control, citus_columnar.control, multi_extension.out and the 14.1-1 -> 14.2-1 migration scripts) were already bumped in 152fdf4 (#8635), so this is a single-file CHANGELOG addition. ### Changelog entries (user-visible only) * #8621 - citus_internal.distribute_object() repair UDF * #8625 - citus.allow_unsafe_insert_select_pushdown GUC * #8566 (via #8694) - skip 2PC for single-statement single-shard procedures * #8587 - object ownership for more citus-internal UDFs * #8638 (via #8653) - re-range sequences when promoting a clone node * #8651 (via #8656) - drop orphaned Citus local table shard copies * #8497 - wrong results when recursive planning projects columns as NULL * #8692 (via #8700) - UPDATE/DELETE with shard key equality + false predicate * #8594 (via #8714) - race condition in shard cleanup * #8498 (via #8715) - type mismatch with COLLATE + type cast * #8556 (via #8716) - segfault in EXPLAIN with LEFT JOIN + subqueries * #8465 (via #8718) - CREATE EXTENSION IF NOT EXISTS ownership error * #8561 (via #8717) - crash on writable standby coordinator CI/test-infra, SQL-plumbing and version-bump commits intentionally excluded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
Release prep for **citus v14.2.0** from `release-14.0`. DESCRIPTION: add changelog for 14.2.0 All version-bearing files (configure.ac, configure, citus.control, citus_columnar.control, multi_extension.out and the 14.1-1 -> 14.2-1 migration scripts) were already bumped in 152fdf4 (#8635), so this is a single-file CHANGELOG addition. ### Changelog entries (user-visible only) * #8621 - citus_internal.distribute_object() repair UDF * #8625 - citus.allow_unsafe_insert_select_pushdown GUC * #8566 (via #8694) - skip 2PC for single-statement single-shard procedures * #8587 - object ownership for more citus-internal UDFs * #8638 (via #8653) - re-range sequences when promoting a clone node * #8651 (via #8656) - drop orphaned Citus local table shard copies * #8497 - wrong results when recursive planning projects columns as NULL * #8692 (via #8700) - UPDATE/DELETE with shard key equality + false predicate * #8594 (via #8714) - race condition in shard cleanup * #8498 (via #8715) - type mismatch with COLLATE + type cast * #8556 (via #8716) - segfault in EXPLAIN with LEFT JOIN + subqueries * #8465 (via #8718) - CREATE EXTENSION IF NOT EXISTS ownership error * #8561 (via #8717) - crash on writable standby coordinator CI/test-infra, SQL-plumbing and version-bump commits intentionally excluded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
Release prep for **citus v14.2.0** from `release-14.0`. This is a **CHANGELOG-only** change. All version-bearing artifacts were already bumped in 152fdf4 (#8635): | artifact | state on `release-14.0` | | --- | --- | | `configure.ac` / `configure` | `14.2.0` | | `citus.control` / `citus_columnar.control` | `14.2-1` | | `expected/multi_extension.out` | references `14.2` | | `14.1-1` -> `14.2-1` migration + downgrade scripts | present | So the diff here is a single file: `CHANGELOG.md | 48 +`. ### Changelog entries (user-visible only) | origin PR | backport PR | entry | | --- | --- | --- | | #8621 | - | `citus_internal.distribute_object()` repair UDF | | #8625 | - | `citus.allow_unsafe_insert_select_pushdown` GUC | | #8566 | #8694 | skip 2PC for single-statement single-shard procedures | | #8587 | - | object ownership for more citus-internal UDFs | | #8638 | #8653 | re-range sequences when promoting a clone node | | #8651 | #8656 | drop orphaned Citus local table shard copies | | #8497 | - | wrong results when recursive planning projects columns as NULL | | #8692 | #8700 | `UPDATE`/`DELETE` with dist-key equality + always-false predicate | | #8594 | #8714 | race condition in shard cleanup (stale catalog snapshot) | | #8498 | #8715 | type mismatch with `COLLATE` + type cast | | #8556 | #8716 | segfault in `EXPLAIN` with `LEFT JOIN` + correlated subqueries | | #8465 | #8718 | `CREATE EXTENSION IF NOT EXISTS` ownership error | | #8561 | #8717 | crash on a writable standby coordinator | Bullets cite **origin** PR numbers only, per the existing convention in the file. ### Commits intentionally excluded Four of the 17 commits in `v14.1.0..release-14.0` carry no `DESCRIPTION:` line and are not user-visible: - `f70aeb54f` - move a GUC test into the N-1-excluded schedule (folds into #8625) - `3383b2463` - `multi_extension` version-ladder test - `65d0a7ece` - `13.3-1--13.4-1` upgrade path plumbing (folds into #8621) - `152fdf425` - the version bump itself (#8635), cited above ### Relationship to #8719 12 of the 13 entries are shared with the 13.4.0 changelog (#8719) and the text is byte-identical between the two, deliberately, so the same fix reads the same way on both lines. The one 14.0-only entry is #8465, which was not backported to 13.2. Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
Fixes #7091.
Closes #7641.
DESCRIPTION: Fixes an unexpected ownership error CREATE EXTENSION IF NOT EXISTS by skipping ownership checks when the extension was already distributed.
Non-owner users receive "must be owner of extension" errors when running
CREATE EXTENSION IF NOT EXISTSon already-distributed extensions, despite PostgreSQL allowing this as a no-op.