yzeng1618 commented on PR #11971: URL: https://github.com/apache/seatunnel/pull/11971#issuecomment-6118935373
> Thanks for keeping the branch in sync. Since the change between `6907e2f2f8fb` and `e1d06f9021c7` looks like a merge from `dev` only, I'd like to confirm the status of the earlier points rather than assume they're closed: > > * **PR11971-F1** – Can you confirm `YashanDbCatalog` no longer closes the shared cached connection when checking for a single-physical-table query? > * **PR11971-F2 / PR11971-F7** – Do the OceanBase/YashanDB overrides now bound the probe query (e.g. a row limit) and reuse a single `ResultSetMetaData`, and does the default catalog path avoid executing the user query twice? > * **PR11971-F3** – Does `mergeWithUnderlyingTable` now load the metadata-verified origin `TablePath` rather than the query-derived identifier? > * **PR11971-F4** – For drivers that report only a schema and no catalog, does the resolved `TablePath` still end up with a null database name? If so, `tableExists`/`getTable` would throw and the merge would silently fall back. Could you either handle the schema-only case explicitly or point me to where it is covered? > * **PR11971-F5** – Is there a changelog entry describing `query_table_metadata_merge` and its `COMMENT` default? If not, please add one. > * **PR11971-F6** – With `ALL`, query-only pipelines gain a merged primary key on upgrade, which affects split planning and sink auto-create DDL. Are the incompatible-changes notes updated for this? Alternatively, if `COMMENT` is intentionally the only behavior applied without opt-in, please state that explicitly in the docs. > * **PR11971-F8** – Are fallback failures now logged at a visible level instead of being swallowed at `DEBUG`? > > A quick pointer to the relevant lines for each would be enough. Once those are confirmed I'm happy to do a final pass. Verified at `e1d06f9021c7`: - **PR11971-F1** — Resolved. `YashanDbCatalog.resolveQueryTable` closes only the `Statement` and `ResultSet`; the shared cached `Connection` remains open. See [YashanDbCatalog.java#L199-L241](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/yashandb/YashanDbCatalog.java#L199-L241). - **PR11971-F2 / PR11971-F7** — Resolved. Both dialect overrides use `setMaxRows(1)` and a single `ResultSetMetaData` for origin verification. The default `AbstractJdbcCatalog` path uses `PreparedStatement.getMetaData()`, so the user query is not executed a second time. See [OceanBaseMySqlCatalog.java#L222-L282](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/oceanbase/OceanBaseMySqlCatalog.java#L222-L282), [YashanDbCatalog.java#L199-L241](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/yashandb/YashanDbCatalog.java#L199-L241), and [AbstractJdbcCatalog.java#L700-L718](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/Abstrac tJdbcCatalog.java#L700-L718). - **PR11971-F3** — Resolved. `mergeWithUnderlyingTable` receives the metadata-verified `singlePhysicalTablePath` separately from the query-derived table and uses the verified path when loading the underlying table. The regression test also uses different query and origin paths. See [JdbcCatalogUtils.java#L549-L583](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtils.java#L549-L583) and [JdbcCatalogUtilsTest.java#L674-L692](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtilsTest.java#L674-L692). - **PR11971-F4** — The catalog-backed path handles schema-only metadata through `withDefaultDatabase`, filling the database from the configured default before calling `tableExists/getTable`. The direct JDBC metadata fallback does not call `tableExists/getTable`; it loads metadata through `CatalogUtils.getCatalogTable`. Tests cover missing database, existing database, and the no-default-database case. See [JdbcCatalogUtils.java#L282-L300](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtils.java#L282-L300), [JdbcCatalogUtils.java#L500-L536](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtils.java#L500-L536), and [JdbcCatalogUtilsTest.java#L785-L808](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-co nnectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtilsTest.java#L785-L808). - **PR11971-F5** — Yes. `query_table_metadata_merge` and its `COMMENT` default are documented in both English and Chinese JDBC documentation. The behavior is also documented in both incompatible-changes documents. See [Jdbc.md#L50](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/docs/en/connectors/source/Jdbc.md#L50), [Jdbc.md#L166](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/docs/en/connectors/source/Jdbc.md#L166), and [incompatible-changes.md#L51-L67](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/docs/en/introduction/concepts/incompatible-changes.md#L51-L67). - **PR11971-F6** — The default `COMMENT` mode only merges comments. Primary keys, constraints, partition keys, and table options remain unchanged by default. `ALL` is an explicit opt-in and its impact on sink DDL, insert/upsert behavior, and split planning is documented. - **PR11971-F8** — Metadata verification failures and underlying-table loading failures are logged at `WARN` with the table path and exception before falling back to the query-derived table. See [AbstractJdbcCatalog.java#L710-L718](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/AbstractJdbcCatalog.java#L710-L718) and [JdbcCatalogUtils.java#L575-L583](https://github.com/yzeng1618/seatunnel-dev/blob/e1d06f9021c7/seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtils.java#L575-L583). The remaining `DEBUG` log in YashanDB is only for optional primary-key enrichment. All eight points are covered at the current head. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
