Skip to content

Conversation

kev-cao
Copy link
Contributor

@kev-cao kev-cao commented Oct 10, 2025

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.

@kev-cao kev-cao requested a review from a team as a code owner October 10, 2025 23:37
@kev-cao kev-cao requested review from jeffswenson and removed request for a team October 10, 2025 23:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@kev-cao kev-cao force-pushed the logical/external-conn-without-db branch 2 times, most recently from 72623cf to 45085b7 Compare October 10, 2025 23:39
@kev-cao kev-cao requested a review from Copilot October 10, 2025 23:41
Copy link

@Copilot Copilot AI left a 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.

@kev-cao kev-cao force-pushed the logical/external-conn-without-db branch 2 times, most recently from bc3f944 to 4445551 Compare October 10, 2025 23:51
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.
@kev-cao kev-cao force-pushed the logical/external-conn-without-db branch from 4445551 to 722ef0d Compare October 14, 2025 15:18
@msbutler msbutler self-requested a review October 20, 2025 14:16
Copy link
Collaborator

@msbutler msbutler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@msbutler msbutler added backport-25.2.x Flags PRs that need to be backported to 25.2 backport-25.3.x Flags PRs that need to be backported to 25.3 backport-25.4.x Flags PRs that need to be backported to 25.4 labels Oct 20, 2025
@msbutler
Copy link
Collaborator

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 20, 2025

@craig craig bot merged commit 3a91af6 into cockroachdb:master Oct 20, 2025
27 checks passed
Copy link

blathers-crl bot commented Oct 20, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-25.2.x Flags PRs that need to be backported to 25.2 backport-25.3.x Flags PRs that need to be backported to 25.3 backport-25.4.x Flags PRs that need to be backported to 25.4 target-release-26.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bidirectional LDR incorrectly requires database name in external connnection

4 participants