Skip to content

[SPARK-52825][SQL] Register existing dialects for additional URL prefixes#57455

Open
cloud-fan wants to merge 6 commits into
apache:masterfrom
cloud-fan:jdbc-dialect-url-aliases
Open

[SPARK-52825][SQL] Register existing dialects for additional URL prefixes#57455
cloud-fan wants to merge 6 commits into
apache:masterfrom
cloud-fan:jdbc-dialect-url-aliases

Conversation

@cloud-fan

@cloud-fan cloud-fan commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR lets users register an existing JdbcDialect for an additional JDBC URL prefix. The
registration affects dialect lookup only; Spark still passes the original URL to the JDBC driver.

It adds JdbcDialects.registerDialectForUrlPrefix and
unregisterDialectForUrlPrefix. It also adds getBuiltInDialect, a case-insensitive lookup for
the built-in mysql, postgresql, db2, sqlserver, derby, oracle, teradata, h2,
snowflake, and databricks dialects. For example, a user can reuse the built-in MySQL dialect
for a known-compatible wrapper:

JdbcDialects.registerDialectForUrlPrefix(
  "jdbc:aws-wrapper:mysql:",
  JdbcDialects.getBuiltInDialect("mysql"))

An unsupported built-in dialect name raises a SparkIllegalArgumentException with the
UNSUPPORTED_BUILT_IN_JDBC_DIALECT error condition.

Prefixes are case-insensitive, validated to start with jdbc: and end with :, and consulted only
when no registered dialect handles the original URL through canHandle. This preserves the
precedence of user-provided dialects. When prefixes overlap, the longest matching prefix wins.

Why are the changes needed?

Wrapper JDBC drivers can use URLs such as jdbc:aws-wrapper:mysql://... while speaking the same
database dialect as the underlying MySQL or PostgreSQL driver.

Matching arbitrary nested protocols with a broad regular expression assumes that every URL of the
form jdbc:<anything>:mysql or jdbc:<anything>:postgresql is compatible with the corresponding
built-in dialect. It can also inspect unrelated portions of the URL. Registering a specific prefix
directly to an existing dialect makes that relationship deliberate and bounded without requiring
users to inherit from or copy a built-in dialect. The named lookup makes all built-in dialects
discoverable without exposing a public collection or requiring knowledge of canonical JDBC URLs.
Spark registers no additional prefixes by default, so compatibility remains an explicit user
decision.

This follows up on the discussion in #53902.

Does this PR introduce any user-facing change?

Yes. Users can look up a built-in dialect by its documented name, and register or unregister an
existing dialect for an additional JDBC URL prefix through JdbcDialects. Wrapper URLs remain
unmatched until a user explicitly registers a dialect for their prefix.

How was this patch tested?

Added tests covering all ten built-in dialect names, case-insensitive named lookup, structured
errors for unsupported names, explicit prefix registration and replacement, case-insensitive
prefix matching, unregistration, prefix validation, longest-prefix matching, user-registered
dialect precedence, and negative cases where unregistered wrappers or database names in the URL
authority must not select a dialect.

The following commands were run:

build/sbt 'sql/testOnly *JDBCSuite -- -z "JDBC dialect"'
build/sbt sql/scalastyle sql/Test/scalastyle

Was this patch authored or co-authored using generative AI tooling?

Generated-by: OpenAI Codex (GPT-5)

@cloud-fan cloud-fan changed the title [SPARK-52825][SQL] Add JDBC dialect URL aliases [SPARK-52825][SQL] Register existing dialects for additional URL prefixes Jul 23, 2026

@viirya viirya 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.

Nice redesign — moving the wrapper-prefix decision out of core and into an explicit user registration cleanly resolves the objection that sank #53902 (baking a specific vendor's URL scheme, and the broad jdbc:(.*:)?mysql.* regex, into the built-in dialects). Spark registers no extra prefixes by default, so compatibility stays an explicit user decision, and this lands on top of the already-reverted clean startsWith("jdbc:mysql") baseline.

I verified the behavior end to end:

  • Precedence is correct. get consults the prefix table only when dialects.filter(_.canHandle(url)) is empty, so user/built-in canHandle dialects always win and a prefix registration can only fill in for URLs nothing else claims. The "registered dialect takes precedence" test pins this.
  • Longest prefix wins, deterministically — registerDialectForUrlPrefix re-sorts by descending prefix length on every insert and getDialectForUrlPrefix takes the first startsWith match, so the result is independent of registration order.
  • Matching stays at the protocol boundary. Prefixes are required to end with : and matching is startsWith, so it can't reach into the authority/path — the exact ambiguity antban raised on #53902 (jdbc:postgresql://host.mysql.com). The "does not inspect the URL authority or path" test locks that down.
  • Case-insensitive normalization and the jdbc:/: validation are consistent between registration and lookup.

One non-blocking note on thread-safety. dialectsByUrlPrefix is an unsynchronized var List mutated by a read-modify-write, so concurrent writers could lose an update and a new registration isn't guaranteed visible to a concurrent reader (no @volatile). This is not a regression — it's exactly the existing contract of registerDialect/unregisterDialect on the same object, which rely on the "register once at setup, on the driver, before running queries" usage pattern; concurrent reads during get are safe because each write swaps in a fresh immutable list. If you ever want to tighten it, wrapping the register/unregister bodies (old and new) in synchronized would close both the lost-update and visibility gaps at negligible cost — but it's equally fine to leave it, since the new methods are deliberately consistent with the existing ones.

Docs and tests are thorough. LGTM.

* Register an existing dialect for an additional JDBC URL prefix. The registration affects
* dialect lookup only; Spark still passes the original URL to the JDBC driver. Registering the
* same prefix again replaces its previous dialect. These registrations are used only when no
* registered dialect handles the URL through [[JdbcDialect.canHandle]].

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.

The scaladoc cross-reference [[JdbcDialect.canHandle]] uses dot notation; genjavadoc/unidoc converts it to {@link JdbcDialect.canHandle}, which javadoc rejects as reference not found (member links need #, not .), hard-failing the "Run / Documentation generation" check (confirmed failing on head 3f20be2). This is the sole doc reference the PR adds.

Fix: use the member-link form [[JdbcDialect#canHandle]] (the convention already used elsewhere in this file, e.g. [[java.sql.ResultSetMetaData#getColumnType]]). Trivial one-line change, but it gates merge until CI is green.

@uros-b

uros-b commented Jul 23, 2026

Copy link
Copy Markdown
Member

Thank you @cloud-fan and @viirya! I left just one more comment, otherwise LGTM

@cloud-fan

cloud-fan commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

cc @aaron-congo @sarutak

@sarutak

sarutak commented Jul 23, 2026

Copy link
Copy Markdown
Member

I'm OK with this approach as a compromise, but I want to note that the design has some usability concerns that ideally should be addressed — particularly the reliance on internal implementation details of non-public dialect classes. Specifically:

1. Discoverability

When a wrapper URL falls through to NoopDialect, there is no warning or guidance pointing the user toward registerDialectForUrlPrefix. The resulting symptoms (type mapping errors, SQL generation issues) don't indicate that dialect selection is the root cause. Users have no way to connect the error they see to the solution this PR provides.

Even once a user finds the API, they need to know what string to pass to JdbcDialects.get() to retrieve the correct built-in dialect. Since the built-in dialects are private case classes (not directly accessible), the only way to obtain one is through JdbcDialects.get() with a URL that triggers canHandle — but the correct input for that (e.g. "jdbc:mysql:") requires knowledge of each dialect's canHandle implementation. This effectively means the API cannot be used without reading the source code. There is also no way for users to discover what built-in dialects are available and which URL prefixes they accept.

It would help to think about how users can discover and use this feature — for example, emitting a warning when falling back to NoopDialect that mentions registerDialectForUrlPrefix and shows a usage hint would at least make the root cause visible and guide users toward the solution.

2. Implicit dependence on canHandle internals

The prefix registration is only consulted when no registered dialect matches via canHandle. This means the behavior of a prefix registration silently depends on the internal implementation of every registered dialect's canHandle. If a future Spark version broadens a canHandle match (or if another registered dialect happens to match the URL), the prefix registration will be silently ignored — no error, no warning. Users cannot reason about whether their registration is effective without understanding all canHandle implementations in play.

@aaron-congo

Copy link
Copy Markdown
Contributor

I agree with @sarutak, I'm generally okay with this approach but he raises some good points that should be considered

@cloud-fan

cloud-fan commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@sarutak Thanks for raising this. I agree that requiring users to call JdbcDialects.get with a
canonical URL makes the built-in dialects hard to discover and leaks knowledge of their
canHandle implementations.

I updated the PR in a732593 to add the case-insensitive
JdbcDialects.getBuiltInDialect(name) API. It supports all ten built-in dialect names: mysql,
postgresql, db2, sqlserver, derby, oracle, teradata, h2, snowflake, and
databricks. This keeps the full set discoverable in the API documentation without exposing a
Scala collection or adding a public constant for every dialect. It returns the exact built-in
instance registered through ServiceLoader. Unsupported names raise a SparkIllegalArgumentException
with the UNSUPPORTED_BUILT_IN_JDBC_DIALECT error condition.

The wrapper registration now reads:

JdbcDialects.registerDialectForUrlPrefix(
  "jdbc:aws-wrapper:mysql:",
  JdbcDialects.getBuiltInDialect("mysql"))

For the precedence concern, I kept the existing behavior deliberately: normal canHandle
registrations continue to win, so adding a prefix alias cannot unexpectedly override a custom or
built-in dialect that explicitly handles the URL. The prefix mapping remains a fallback only for
otherwise-unhandled URLs. This ordering is documented in the API and covered by the precedence
test.

@cloud-fan
cloud-fan force-pushed the jdbc-dialect-url-aliases branch from a201581 to 5ff6fd8 Compare July 24, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants