Skip to content

[#13945] fix(jdbc): Honor pool wait settings and validate MySQL table properties - #13946

Merged
yuqi1129 merged 6 commits into
apache:mainfrom
yuqi1129:fix/jdbc-property-validation
Oct 20, 2026
Merged

yuqi1129 merged 6 commits into
apache:mainfrom
yuqi1129:fix/jdbc-property-validation

Conversation

@yuqi1129

@yuqi1129 yuqi1129 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Register and forward jdbc.pool.max-wait-ms so it reaches the JDBC connection pool. As with jdbc.pool.max-idle, an explicit canonical value takes precedence over gravitino.bypass.maxWaitMillis; the canonical default no longer overrides a bypassed maxWaitMillis.
  • Reject undeclared MySQL table properties instead of silently dropping them, while retaining internal identifier and credential metadata handling.
  • Decode supported MySQL options using their declared enum/integer types and render canonical SQL values. Apply the same validation to create/alter SQL generation.
  • Remove Spark's implicit owner metadata in the JDBC connector before forwarding table options, preserving normal Spark table creation under strict MySQL validation.
  • Strip the JDBC scan options the Flink connector injects (url, table-name, username, password) before forwarding table options, so CREATE TABLE ... LIKE keeps working and the catalog password is no longer sent as a table property. Same-named options written explicitly in WITH are dropped too; they never took effect, because scan options are always regenerated from the catalog connection.
  • Make the shared Flink catalog ITs send arbitrary WITH options only to catalogs that accept custom table properties; the MySQL Flink IT opts out.
  • Clarify the MySQL property names and immutable-property policy in the catalog documentation, and list the supported keys in the rejection message.
  • Document jdbc.pool.max-wait-ms and its bypass precedence in every JDBC catalog page (adds the missing Hologres row).

Why are the changes needed?

The canonical pool-wait setting currently falls back to 30000 ms. Unknown MySQL table properties are accepted and then discarded by mapping on both create and alter. This patch corrects these two property-handling paths; it does not claim that unknown properties bypass immutability through the public API.

Fix: #13945

Does this PR introduce any user-facing change?

Configured pool wait times take effect, whether set through jdbc.pool.max-wait-ms or gravitino.bypass.maxWaitMillis. Values that were previously ignored now apply after upgrade: a stored non-positive jdbc.pool.max-wait-ms fails catalog initialization, and a bypassed maxWaitMillis=-1 means an unbounded wait. MySQL create/alter requests with unknown table properties now fail with an error naming the key. Supported values are validated before DDL. Shared metadata keys that every entity accepts (the internal identifier and credential-vending keys) are still accepted and not turned into MySQL options, since they are not table options. engine and auto-increment-offset remain immutable, and strict unknown-key validation is scoped to MySQL. Spark JDBC no longer forwards its reserved owner metadata as a table option; other user-supplied keys continue to reach the server for catalog-specific handling.

How was this patch tested?

  • Full JDBC common and MySQL non-Docker unit suites: 106 and 29 tests passed.
  • Regressions exercise catalog initialization through the actual pool, catalog create/alter property mapping, internal metadata handling, and create/alter SQL generation before any connection is borrowed.
  • Separate mutation checks revert the pool registration, MySQL metadata validation, and SQL validation; the corresponding regression cases fail as expected.
  • MySQL catalog module with Docker enabled: 51 tests passed, including real create/alter property round trips.
  • Full Spark 3.5 and 4.0 non-Docker suites: 233 and 229 tests passed, including implicit-owner removal and preservation of unknown user keys for server-side validation.
  • Reverting Spark owner removal makes both owner regression cases fail; restoring the fix passes all four converter tests.
  • Pool wait precedence covers canonical, prefixed canonical and DBCP maxWaitMillis bypass combinations (6 cases); reverting the precedence guard fails the bypass-only case.
  • Flink converter suites (MySQL, PostgreSQL) cover stripping injected scan options; removing the stripping fails both. A new CREATE TABLE ... LIKE case in FlinkJdbcMysqlCatalogIT119 fails without the fix (Unsupported MySQL table property: password) and passes with it.
  • FlinkJdbcMysqlCatalogIT119 with Docker MySQL: 30 tests, 0 failures, including the LIKE case and the four create-table cases that previously sent unsupported options.
  • JDBC common, MySQL, and Spark common, and Flink common Spotless checks.

Copilot AI balanced review requested due to automatic review settings October 19, 2026 09:05

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Code Coverage Report

Overall Project 72.33% +0.19% 🟢
Files changed 93.07% 🟢

Module Coverage
aliyun 19.74% 🔴
api 55.09% 🟢
authorization-common 85.96% 🟢
authorization-ranger 14.44% 🔴
aws 60.78% 🟢
aws-bundle 0.0% 🔴
azure 68.04% 🟢
azure-bundle 0.0% 🔴
catalog-common 29.49% 🔴
catalog-fileset 82.37% 🟢
catalog-glue 73.24% 🟢
catalog-hive 84.5% 🟢
catalog-jdbc-common 58.22% +5.33% 🟢
catalog-jdbc-doris 87.89% 🟢
catalog-jdbc-mysql 84.91% +14.16% 🟢
catalog-jdbc-postgresql 83.76% 🟢
catalog-jdbc-starrocks 79.44% 🟢
catalog-kafka 75.21% 🟢
catalog-lakehouse-generic 86.91% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 86.8% 🟢
catalog-lakehouse-paimon 84.36% 🟢
catalog-model 77.99% 🟢
cli 44.62% 🟢
client-java 79.44% 🟢
client-java-runtime 0.0% 🔴
common 60.9% 🟢
core 87.57% 🟢
filesystem-hadoop3 76.89% 🟢
flink 55.56% 🟢
flink-common 32.39% +9.55% 🔴
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 17.84% 🔴
hive-metastore-common 62.3% 🟢
hive-metastore3-libs 58.8% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 67.14% 🟢
iceberg-rest-server 77.45% 🟢
idp-basic 86.99% 🟢
integration-test-common 0.0% 🔴
jobs 71.35% 🟢
k8s-job-executor 97.81% +0.29% 🟢
lance-common 40.73% 🟢
lance-rest-server 68.04% 🟢
lineage 59.39% 🟢
optimizer 84.27% 🟢
optimizer-api 46.58% 🟢
server 91.44% 🟢
server-common 84.0% 🟢
spark 63.35% 🟢
tencent 81.78% 🟢
tms 80.04% 🟢
trino-connector 64.88% 🟢
Files
Module File Coverage
catalog-jdbc-common JdbcCatalogPropertiesMetadata.java 100.0% 🟢
DataSourceUtils.java 92.5% 🟢
catalog-jdbc-mysql MysqlTablePropertiesMetadata.java 100.0% 🟢
MysqlTableOperations.java 87.55% 🟢
flink-common JdbcPropertiesConverter.java 97.78% 🟢
k8s-job-executor SparkApplicationUtils.java 100.0% 🟢
spark-common JdbcPropertiesConverter.java 93.1% 🟢

MySQL now rejects table properties it does not support, so the shared
Flink ITs only send arbitrary WITH options to catalogs that accept them.
Apply the canonical pool wait only when it is configured or no DBCP
maxWaitMillis bypass is present, matching maxIdle. List the supported
keys in MySQL table property errors and document the pool wait setting
and its precedence across the JDBC catalog docs.
@yuqi1129 yuqi1129 self-assigned this Oct 20, 2026
@yuqi1129 yuqi1129 added the branch-1.3 Automatically cherry-pick commit to branch-1.3 label Oct 20, 2026
CREATE TABLE ... LIKE copies the url, table-name, username and password
options the connector adds for scans, and MySQL now rejects them as
table properties. Strip them before forwarding table options, as Spark
does for owner, and note that shared metadata keys are not applied to
MySQL tables.

@diqiu50 diqiu50 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

@yuqi1129
yuqi1129 merged commit 3ce9197 into apache:main Oct 20, 2026
44 of 45 checks passed
yuqi1129 added a commit that referenced this pull request Oct 21, 2026
…ngs and validate MySQL table properties (#13946) (#14035)

### What changes were proposed in this pull request?

Backport commit `3ce91977a2d137af39321716a9fb5813922afceb` (#13946) to
`branch-1.3`: honor JDBC pool wait settings and validate MySQL table
properties.

Resolve the committed conflict markers while retaining the 1.3
validation query, connection timeout handling and Spark
`getPGInstance()` API. Adapt tests to the existing 1.3 ALTER connection
lifecycle. Pool max-idle and table credential-vending settings are not
introduced by this backport.

### Why are the changes needed?

Configured pool wait values could be lost or overwritten, and
unsupported MySQL table options could reach generated SQL. This backport
brings the fixes to 1.3.

### Does this PR introduce _any_ user-facing change?

Yes. JDBC pool wait settings are honored, and unsupported or invalid
MySQL table options are rejected before DDL is executed.

### How was this patch tested?

- 70 targeted non-Docker tests passed across JDBC common, MySQL, Flink
common and Spark common.
- Formatting and conflict-marker checks passed.
- Six mutation cases fail if the retained `SELECT 1` pool validation
query is removed, then pass after restoration.
- Docker tests were not run locally because the Docker daemon is
unavailable; CI validation is pending.

---------

Co-authored-by: Qi Yu <yuqi@datastrato.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch-1.3 Automatically cherry-pick commit to branch-1.3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug report] JDBC pool wait settings and MySQL table properties are silently ignored

3 participants