Repository navigation
[Cherry-pick to branch-1.3] [#13945] fix(jdbc): Honor pool wait settings and validate MySQL table properties (#13946) - #14035
Merged
Conversation
… properties (#13946) ### 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. # Conflicts: # catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogPropertiesMetadata.java # catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java # catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogPropertiesMetadata.java # catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java # docs/jdbc-clickhouse-catalog.md # docs/jdbc-doris-catalog.md # docs/jdbc-hologres-catalog.md # docs/jdbc-mysql-catalog.md # docs/jdbc-oceanbase-catalog.md # docs/jdbc-postgresql-catalog.md # docs/jdbc-starrocks-catalog.md # spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/jdbc/TestJdbcPropertiesConverter.java
github-actions
Bot
added
cherry-pick
yuqi1129
removed
the
cherry-pick-conflict
yuqi1129
changed the title
Author
Code Coverage Report
Files
|
yuqi1129
approved these changes
Oct 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Backport commit
3ce91977a2d137af39321716a9fb5813922afceb(#13946) tobranch-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?
SELECT 1pool validation query is removed, then pass after restoration.