DanielLeens commented on PR #11971: URL: https://github.com/apache/seatunnel/pull/11971#issuecomment-6338505715
Thanks @SEZ9. I checked your F1-F8 summary against `4d4a6ecf`, because these have changed from the earlier implementation: - **F1/F2:** `YashanDbCatalog.resolveQueryTable` keeps the cached connection outside try-with-resources (206), caps returned rows with setMaxRows(1) (208), and closes the statement/result set only. OceanBaseMySqlCatalog does the same at 234-243. A returned-row cap does not promise that the database avoids all expensive query planning/scanning, but neither path now materializes an unbounded result just to consume metadata. - **F3/F4/F7:** `JdbcCatalogUtils.java:281-293` consumes one QueryTableResolution, loads its verified physical path, and fills a blank database through `withDefaultDatabase` (526-531). It does not discard that path and reconstruct it from the query-derived identifier. The existing unit cases around 668 and 787 pin verified-origin and default-database handling. - **F8:** verification fallback WARN is at `AbstractJdbcCatalog.java:710-711`; load fallback WARN is at `JdbcCatalogUtils.java:580`. These are not DEBUG-only on this head. - **F5/F6:** EN Jdbc.md:50/166 and incompatible-changes.md:150 onward document that COMMENT is the default, preserving primary keys/options/constraints/partition keys, while ALL is explicit opt-in. The ZH mirrors document the same. Therefore unconditional new primary-key inference is not the default behavior of the final implementation. The PR description still contains the older broad merge description and would benefit from being aligned with COMMENT versus ALL; that is not evidence the old implementation is still running. My prior note about table_path plus query taking precedence over the query-only mode remains a non-blocking scope clarification. This reply does not close your review on your behalf or constitute another full code review. Build is green, but GitHub now reports conflicts against dev. Please resolve those conflicts and rerun checks on the new head; the full updated diff and final write-capable maintainer review will then need a fresh pass. Current status evidence: head `4d4a6ecfca200e39e2d489d1b233bba2948f027d`, base `dev`, compare `diverged` (ahead 12, behind 87), mergeable state `dirty`. -- 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]
