[opt](jdbc) Support Oracle BINARY_FLOAT and BINARY_DOUBLE types - #68200
zhangstar333 wants to merge 1 commit into
Conversation
|
/review |
There was a problem hiding this comment.
Requesting changes for three correctness issues.
Review status: capped/incomplete. The third and final allowed convergence round surfaced the non-finite predicate issue below. Every discovered candidate was adjudicated, but the review contract forbids a fourth round, so this review cannot claim convergence.
Critical checkpoints:
- Goal and test proof: not achieved. Normal
type='jdbc'catalogs execute the separate plugin mapper, which remains unsupported; the added unit test directly proves only the inactive legacy switch. No Oracle end-to-end schema/read/write/predicate result was supplied. - Scope and parallel paths: the local switch edit is small, but it misses the active plugin metadata path and its downstream write and predicate behavior.
- Concurrency, locking, lifecycle/static initialization, persistence/EditLog, transaction atomicity, and observability: no new applicable issue found.
- Configuration and compatibility: existing FLOAT/DOUBLE descriptor transport and old/new Oracle scan getters are compatible. The sink, however, lacks the Oracle/remote-type information needed for native binary binding. No new storage-format or rolling-upgrade issue was found.
- Special conditions: NULL and finite read materialization are already supported; NaN/infinity predicate serialization is not.
- Performance: the mapper itself is negligible, while Oracle documents avoidable conversion work in the current NUMBER-format write binding.
- User focus: no additional focus was provided.
Validation was static-only as required by the review prompt; I did not run builds or tests, and author/CI claims were not treated as independent validation. Reviewed exact head 097a9331263580ca645df028c07b8994fa888369.
| precision = scale; | ||
| } | ||
| return createDecimalOrStringType(precision, scale); | ||
| case "BINARY_FLOAT": |
There was a problem hiding this comment.
[P1] Update the Oracle connector mapper used by JDBC catalogs
A normal type='jdbc' catalog is routed through JdbcConnectorProvider to JdbcOracleConnectorClient, and JdbcConnectorMetadata#getTableSchema calls that class's jdbcTypeToConnectorType. Its switch still has only FLOAT and falls through to UNSUPPORTED for both BINARY_FLOAT and BINARY_DOUBLE; this legacy JdbcOracleClient mapper is not on the catalog path (its remaining production callers are streaming helpers whose source enum excludes Oracle). Consequently this unit test can pass while DESC/queries on an Oracle catalog still reject these columns. Please add the two mappings to JdbcOracleConnectorClient and cover that production mapper, ideally with the existing Oracle JDBC regression suite.
| } | ||
| return createDecimalOrStringType(precision, scale); | ||
| case "BINARY_FLOAT": | ||
| return Type.FLOAT; |
There was a problem hiding this comment.
[P1] Do not push bare NaN/Infinity tokens to Oracle
After the production mapping is fixed, these types make non-finite predicates reachable. The connector currently unwraps CAST, carries the Java Double into JdbcQueryBuilder, and literalToSql renders it with toString(), so for example binary_double_col < CAST('Infinity' AS DOUBLE) becomes "BD" < Infinity. Oracle accepts the supplied BINARY_FLOAT_NAN/BINARY_DOUBLE_NAN and BINARY_FLOAT_INFINITY/BINARY_DOUBLE_INFINITY constants, not bare NaN/Infinity tokens (Oracle literals). Please render the type-appropriate Oracle literal (including negative infinity) or decline this conjunct from pushdown so BE evaluates it locally, and add query-builder/regression coverage.
| return createDecimalOrStringType(precision, scale); | ||
| case "BINARY_FLOAT": | ||
| return Type.FLOAT; | ||
| case "BINARY_DOUBLE": |
There was a problem hiding this comment.
[P1] Preserve the binary format on Oracle JDBC writes
Once these cases are added to the production connector mapper, the columns also become writable through the JDBC catalog. The sink currently binds every Doris FLOAT/DOUBLE with standard PreparedStatement.setFloat/setDouble, and its writer parameters carry neither the remote Oracle type nor even table_type. Oracle documents that these standard setters send NUMBER-format bits by default; converting those bits back into BINARY_FLOAT/BINARY_DOUBLE is not bit-exact and can corrupt values (OraclePreparedStatement). Please preserve enough Oracle/type information to use native binary setters, or enable SetFloatAndDoubleUseBinary for Oracle writer connections, and cover a binary-float INSERT/readback round trip.
What problem does this PR solve?
Problem Summary:
Support Oracle
BINARY_FLOATandBINARY_DOUBLEtype mappingRelease note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)