Skip to content

Accept JDBC connection string starting with "jdbc:spanner" used by Spanner Omni#3559

Merged
feeblefakie merged 5 commits into
masterfrom
accept_spanner_jdbc_con_str
May 18, 2026
Merged

Accept JDBC connection string starting with "jdbc:spanner" used by Spanner Omni#3559
feeblefakie merged 5 commits into
masterfrom
accept_spanner_jdbc_con_str

Conversation

@Torch3333

@Torch3333 Torch3333 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Description

Google Cloud Spanner uses a JDBC connection string starting with jdbc:cloudspanner: but Spanner Omni documentation uses a connection string starting with jdbc:spanner:. Currently, ScalarDB only accepts the former and rejects the latter. This PR makes sure both connection string patterns are accepted.

Note

Using a jdbc:cloudspanner: connection string with Spanner Omni is possible.

Related issues and/or PRs

Changes made

  • Update RdbEngineFactory to accept jdbc:spanner: as a valid connection string pattern for Spanner.

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes.
  • I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
  • Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
  • Tests (unit, integration, etc.) have been added for the changes.
  • My changes generate no new warnings.
  • Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release notes

Accepts connection string starting with jdbc:spanner to connect to Spanner (emulator, omni, and cloud instance) in addition to the already supported jdbc:cloudspanner pattern.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the RdbEngineFactory to recognize the jdbc:spanner JDBC URL prefix, enabling support for Spanner Omni. A review comment suggests appending a trailing colon to the new prefix to maintain consistency with other database protocols and prevent potential false matches.

Comment thread core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineFactory.java Outdated
@Torch3333 Torch3333 force-pushed the accept_spanner_jdbc_con_str branch from 5b99a95 to 15fb890 Compare May 7, 2026 06:13
@Torch3333 Torch3333 self-assigned this May 7, 2026
@Torch3333 Torch3333 marked this pull request as ready for review May 7, 2026 06:23
@Torch3333 Torch3333 requested review from a team, KodaiD, brfrn169, Copilot, feeblefakie and komamitsu and removed request for a team May 7, 2026 06:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates ScalarDB’s JDBC engine selection to recognize Spanner JDBC URLs that start with jdbc:spanner: (used by Spanner Omni), in addition to the existing jdbc:cloudspanner: prefix, so Spanner connections aren’t rejected based solely on URL scheme.

Changes:

  • Extend RdbEngineFactory URL prefix matching to treat jdbc:spanner: as Spanner.
  • Keep Spanner engine selection behavior unchanged otherwise (still routes to RdbEngineSpanner).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineFactory.java
Comment thread core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineFactory.java
Comment thread core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineFactory.java
@Torch3333 Torch3333 changed the title Accepts JDBC connection string starting with "jdbc:spanner" used by Spanner Omni Accept JDBC connection string starting with "jdbc:spanner" used by Spanner Omni May 7, 2026

@komamitsu komamitsu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! 👍

@brfrn169 brfrn169 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Thank you!

@KodaiD KodaiD left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thank you!

@feeblefakie feeblefakie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thank you!

@feeblefakie feeblefakie enabled auto-merge (squash) May 18, 2026 05:49
@feeblefakie feeblefakie merged commit 68fcc1c into master May 18, 2026
59 of 61 checks passed
@feeblefakie feeblefakie deleted the accept_spanner_jdbc_con_str branch May 18, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants