-
Notifications
You must be signed in to change notification settings - Fork 4k
logical: external conn should not require db name #155292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
logical: external conn should not require db name #155292
Conversation
72623cf
to
45085b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where LDR (Logical Data Replication) incorrectly required a database name in external connection URIs when setting up bidirectional streams. The fix addresses privilege lookup problems during reverse stream setup by using fully qualified table names.
- Changed table name resolution to use fully qualified names instead of unqualified names
- Added test coverage to verify external connections work without database names in the URI
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
pkg/crosscluster/logical/create_logical_replication_stmt.go | Updated TargetTableNames() to return fully qualified table names using FQString() instead of Table() |
pkg/crosscluster/logical/logical_replication_job_test.go | Added new test to verify bidirectional replication works without database name in external connection URI |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bc3f944
to
4445551
Compare
LDR currently erroneously requires the db name in the external connection with setting up a bidirectional connection. This was determined to be caused by the fact that the privilege lookup on the source cluster was using unqualified table names during the reverse stream setup. This commit fixes the issue by using fully qualified table names. Fixes: cockroachdb#152395 Release note: LDR no longer requires the database name to be specified in the external connection URI when setting up a bidirectional stream.
4445551
to
722ef0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
bors r+ |
Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches. Issue #152395: branch-release-25.2, branch-release-25.3, branch-release-25.4. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
LDR currently erroneously requires the db name in the external connection with setting up a bidirectional connection. This was determined to be caused by the fact that the privilege lookup on the source cluster was using unqualified table names during the reverse stream setup. This commit fixes the issue by using fully qualified table names.
Fixes: #152395
Release note: LDR no longer requires the database name to be specified in the external connection URI when setting up a bidirectional stream.