schemastore: fix cross-schema create view with unqualified source table#5027
schemastore: fix cross-schema create view with unqualified source table#5027lidezhu wants to merge 6 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
ti-chi-bot
Bot
added
release-note
📝 WalkthroughWalkthroughThis PR fixes TiCDC's replication of CREATE VIEW statements that reference unqualified table names across database boundaries. It adds schema extraction utilities, normalization logic to restore database qualifiers in view DDL, and integration tests to validate the fix. ChangesCREATE VIEW Normalization for Cross-Schema References
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Command failed Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
ti-chi-bot
Bot
added
the
size/S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces integration tests for cross-schema views by creating a table in one database and a corresponding view in another. It updates the test configuration and execution scripts to verify these objects in the downstream environment. A reviewer suggested enhancing the test by inserting sample data into the source table to verify that data is correctly replicated and accessible through the cross-schema view.
ti-chi-bot
Bot
added
size/M
lidezhu
changed the title
ti-chi-bot
Bot
added
size/L
ti-chi-bot
Bot
removed
the
do-not-merge/work-in-progress
|
/gemini review |
|
@coderabbitai review |
|
/gemini summary |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request implements normalization for CREATE VIEW DDL events by utilizing TiDB's stored SelectStmt to ensure table references are fully resolved, particularly for cross-schema views. It introduces a new AST visitor to extract schema names and includes comprehensive unit and integration tests. Feedback was provided to improve the efficiency of the normalization logic by reusing a single parser instance for multiple parsing operations within the same function.
Summary of ChangesThis pull request addresses an issue where cross-schema CREATE VIEW statements could fail or be incorrectly processed due to unqualified source table references. By leveraging the normalized SELECT statement stored in the table metadata, the system can now correctly reconstruct the view definition with fully qualified table names. This ensures consistency and accuracy in downstream replication for views that span multiple schemas. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Activity
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration_tests/common_1/run.sh`:
- Around line 68-69: The two calls to check_table_exists use unquoted expansions
which can break on word-splitting/globbing; update the calls to quote each
argument so they pass literal table names and host/port safely, e.g. call
check_table_exists "common_1.users" "$DOWN_TIDB_HOST" "$DOWN_TIDB_PORT" and
check_table_exists "common.v" "$DOWN_TIDB_HOST" "$DOWN_TIDB_PORT" (identify the
two occurrences of check_table_exists in the diff and add quotes around the
table names and the $DOWN_TIDB_HOST/$DOWN_TIDB_PORT expansions).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92823661-0c11-449d-a2a3-fcadbb56e18b
📒 Files selected for processing (7)
logservice/schemastore/persist_storage_ddl_handlers.gologservice/schemastore/persist_storage_test.gologservice/schemastore/utils.gologservice/schemastore/utils_test.gotests/integration_tests/common_1/data/test.sqltests/integration_tests/common_1/data/test_finish.sqltests/integration_tests/common_1/run.sh
|
/test all |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3AceShowHand The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
2 participants
What problem does this PR solve?
Issue Number: close #5026
What is changed and how it works?
This pull request addresses an issue where cross-schema CREATE VIEW statements could fail or be incorrectly processed due to unqualified source table references. By leveraging the normalized SELECT statement stored in the table metadata, the system can now correctly reconstruct the view definition with fully qualified table names. This ensures consistency and accuracy in downstream replication for views that span multiple schemas.
Highlights
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note
Summary by CodeRabbit
New Features
Tests