Skip to content

fix(redshift-alpha): align qualified table names - #39304

Open
ozelalisen wants to merge 1 commit into
aws:mainfrom
ozelalisen:redshift-alpha-qualified-table-names
Open

ozelalisen wants to merge 1 commit into
aws:mainfrom
ozelalisen:redshift-alpha-qualified-table-names

Conversation

@ozelalisen

Copy link
Copy Markdown
Member

Reason for this change

Since 2.262.0, Table treats a dotted tableName as one identifier, but grants treat it as schema.table. A grant can then address a different table.

Description of changes

  • Quote each component of a dotted tableName separately in every table statement.
  • Complete the rollback of a rejected qualifier change or empty-component name without running statements, for tables created after 2.114.1 that:
    • keep their created qualifier;
    • have no empty component in their created or current name;
    • had no colon in their name, cluster or database at creation.
  • Those rollbacks still run as ordinary updates for other tables, and can fail again.
  • An update back to a table's original qualifier can be treated as a rollback and apply nothing, if an earlier release renamed the table without keeping its qualifier. For example, renaming analytics.orders to orders2 left it as analytics.orders2. This change doesn't repair such tables.

Describe any new or updated permissions being added

Production permissions are unchanged. In integ.database, a test function can run and read a Redshift Data API query with the administrator secret and write logs, and the assertion can invoke it.

Description of how you validated changes

  • Build, lint and 358 unit tests pass.
  • Regenerated the six integ snapshots that embed the query provider; outside integ.database, their templates change only in its asset hash. integ.database now uses public.IntegTable and asserts the user's privileges on it.
  • On a development account, builds with this change:
    • completed both rollbacks without running statements;
    • deployed and passed five of the six integ tests, including the integ.database assertion. integ.cluster-distkey could not finish, because it needs more Elastic IP addresses than the account allows.

Checklist

BREAKING CHANGE: Table now interprets dots in tableName as schema.table or database.schema.table for every table operation, as grants already did. Tables created with 2.262.0 or later from a dotted tableName exist as one identifier (for example "analytics.orders" in the default schema); after upgrading, updates and RemovalPolicy.DESTROY deletion address analytics.orders instead. Names with empty components now fail at synthesis, and renames cannot add, remove or change the qualifier.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions Bot added the p2 label Oct 15, 2026
@aws-cdk-automation
aws-cdk-automation requested a review from a team October 15, 2026 14:49
@mergify mergify Bot added the contribution/core This is a PR that came from AWS. label Oct 15, 2026
@mergify
mergify Bot deployed to automation October 15, 2026 14:50 Active
@mergify
mergify Bot deployed to automation October 15, 2026 14:50 Active
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ This pull request description does not follow the correct template structure.

PRs without a linked issue will receive lower priority for review and merging. Please update the description to follow the PR template and include a line like Closes #123 in the Issue section. If no existing issue matches your change, create one first.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
This security report is NOT a review blocker. Please try merge from main to avoid findings unrelated to the PR.
To suppress a specific rule, see Suppressing Rules.


TestsPassed ✅SkippedFailed
Security Guardian Results192 ran192 passed
TestResult
No test annotations available

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
This security report is NOT a review blocker. Please try merge from main to avoid findings unrelated to the PR.
To suppress a specific rule, see Suppressing Rules.


TestsPassed ✅SkippedFailed
Security Guardian Results with resolved templates192 ran192 passed
TestResult
No test annotations available

@maintainer-for-aws

maintainer-for-aws Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Automated review

A maintainer will still review this — treat the notes below as a starting point.

📌 Commit 045592a

This PR (fix(redshift-alpha): align qualified table names) fixes a divergence in the experimental @aws-cdk/aws-redshift-alpha module where the custom-resource table handler quoted a dotted tableName as a single identifier ("public.orders") while the sibling privileges handler already treated it as the schema-qualified relation public.orders. As a result a table and its grant could resolve to different relations. The change switches the table handler to quoteQualifiedIdentifier across create/drop/comment/all ALTER variants, makes renames operate on the leaf only (with a guard rejecting cross-qualifier moves), rejects empty dotted components at synth and runtime, and adds a conservative rollback no-op path for CloudFormation's auto-rollback of a rejected update. Supporting changes add schema-qualified doc notes on tableName, a README note and comma fix, an eslint-suppression count bump, extensive unit coverage, and an IntegTest-based deploy-time assertion proving the grant and table now resolve to the same relation.

This is a deliberate breaking change, confined to an experimental module and carrying an explicit BREAKING CHANGE: callout, so it is permitted. The empty-component rejection and new rename guards are documented parts of that break rather than false positives, the synth-time check is correctly gated by token resolution, and the rollback heuristic only short-circuits a path that would otherwise fail. The public surface stays string-typed and idiomatic to Redshift's own schema.table notation, the test coverage exercises each new behavior with assertions that would turn red on regression, and no new security exposure is introduced. No issues found on this change.

🔴 0 blocking · 🟡 0 recommended · ⚪ 0 optional


Generated automatically. React 👍 or 👎 to tell us whether this review helped, so we can improve these reviews.

@aws-cdk-automation aws-cdk-automation added the pr/needs-further-review PR requires additional review from our team specialists due to the scope or complexity of changes. label Oct 15, 2026
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 15, 2026
Since 2.262.0, Table treats a dotted tableName as one identifier, but grants treat it as schema.table. A grant can then address a different table.

- Quote each component of a dotted tableName separately in every table statement.
- Complete the rollback of a rejected qualifier change or empty-component name without running statements, for tables created after 2.114.1 that:
  - keep their created qualifier;
  - have no empty component in their created or current name;
  - had no colon in their name, cluster or database at creation.
- Those rollbacks still run as ordinary updates for other tables, and can fail again.
- An update back to a table's original qualifier can be treated as a rollback and apply nothing, if an earlier release renamed the table without keeping its qualifier. For example, renaming analytics.orders to orders2 left it as analytics.orders2. This change doesn't repair such tables.

Validation:

- Build, lint and 378 unit tests pass.
- Regenerated the six integ snapshots that embed the query provider; outside integ.database, their templates change only in its asset hash. integ.database now uses public.IntegTable and asserts the user's privileges on it.
- On a development account, builds with this change:
  - completed both rollbacks without running statements;
  - deployed and passed five of the six integ tests, including the integ.database assertion. integ.cluster-distkey could not finish, because it needs more Elastic IP addresses than the account allows.

BREAKING CHANGE: Table now interprets dots in tableName as schema.table or database.schema.table for every table operation, as grants already did. Tables created with 2.262.0 or later from a dotted tableName exist as one identifier, for example "analytics.orders" in the default schema; after upgrading, updates and RemovalPolicy.DESTROY deletion address analytics.orders instead. Names with empty components now fail at synthesis, and renames cannot add, remove or change the qualifier.
@ozelalisen
ozelalisen force-pushed the redshift-alpha-qualified-table-names branch from d4de676 to 045592a Compare October 17, 2026 14:07

This branch was successfully deployed

1 active deployment
automation — 045592ab Deployed Oct 17, 2026 by ozelalisen via validate-pr #373079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS. p2 pr/needs-further-review PR requires additional review from our team specialists due to the scope or complexity of changes. pr/needs-maintainer-review This PR needs a review from a Core Team Member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants