urosstan-db commented on PR #59990: URL: https://github.com/apache/spark/pull/59990#issuecomment-6558990658
**Review: no blocking findings were identified.** ## Review summary No defects were found in this round. `JDBCTable.mergeOptions` keeps the existing raw, case-sensitive merge of table and per-query options, so distinct-case driver properties still reach the driver unchanged. It then writes a single entry for each of `timestampNTZAsWallClock` and `zonelessTimestampAsNTZ`: the per-query value when the query supplies one, matched case-insensitively, and otherwise the value the table resolved when it was loaded. The scan builder, the write builder, and the options rebuilt for JDBC join pushdown all read this map. A V2 Oracle table whose schema was resolved with `spark.sql.legacy.oracle.timestampNTZMapping.enabled` off therefore keeps its wall-clock NTZ conversion when the flag is turned on before the scan or write is built. Non-Oracle dialects, and sessions that leave the flag unchanged, get the same values as on the base branch. The new `JDBCTableCatalogSuite` grid builds the table under one flag value and both builders under the other, in both directions, and checks both builders. Its cases with no catalog or query value fail on the base branch. The driver-property test guards the preserved raw merge. The Docker V2 test covers an end-to-end read after a flag flip, but as the description notes, the Oracle Docker suites were only compiled locally. Both test nits from the previous review are addressed. The V1 Oracle Docker suite now creates `USING jdbc` tables with non-default `zonelessTimestampAsNTZ` and `timestampNTZAsWallClock` OPTIONS, so those checks fail if the OPTIONS are ignored. `JDBCSuite` again asserts that an `AggregatedDialect` with no opting member reports `timestampNTZAsWallClock = false`. This revision also answers that review's design question: it keeps the session-level legacy flag and makes the V2 builders inherit the values resolved when the table is loaded. This review comment was written by Isaac. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
