Repository navigation
Conversation
Gabriel39
requested review from
924060929,
CalvinKirs,
dataroaring,
englefly,
morningman,
morrySnow and
starocean999
as code owners
October 8, 2026 09:49
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review complete at head 01eb76566c253ae33b1d76a459d9ad01c28ddea8 across all 217 changed paths. I request changes for the four inline findings: two P1 correctness/test blockers and two P2 query/reader-selection defects. There were no existing inline comments to carry forward.
Critical checkpoint conclusions:
- Goal and tests: The change makes external VARBINARY and TIMESTAMPTZ mappings consistent across connectors and adds unit and regression coverage. The new Iceberg wire test cannot exercise its intended NULL marker (P1); the Java JDBC bind tests inject dialect names directly and miss the BE parameter mismatch (P1).
- Scope: The large connector, FE, BE, and golden-result surface follows the mapping change. No unrelated change was substantiated during the file sweep.
- Concurrency: Catalog marker migration runs after fenced replay under the catalog manager write lock; journal-before-local-replay ordering was traced. No new thread-safety or lock-order defect was substantiated.
- Lifecycle: Catalog promotion/rebuild, JNI scanner and writer selection, Paimon native/JNI dispatch, and Iceberg partition write/commit lifecycles were traced. No separate ownership, cleanup, or static-initialization defect was found.
- Configuration: Catalog mapping markers are migrated and enforced as true; the legacy ORC LTZ table option is checked during scan planning. The latter check is too broad for unprojected LTZ fields (P2). No new runtime-updatable setting was identified.
- Compatibility: Production Iceberg FE uses Thrift field 20 for NULL keys, while the new manual test writes field 19 (P1). The old-FE/new-BE Iceberg FIXED-write gap is explicitly documented as an accepted rolling-upgrade limitation, so it is not reposted.
- Parallel paths: Static/dynamic Iceberg partitions, JDBC table scans/TVFs and writes, and Paimon native/JNI reads were compared. The TVF wrapper breaks a valid terminal line comment (P2); the JDBC write path sends a numeric dialect where its Java factory expects a name (P1).
- Conditions: Binary and timestamp type guards and historical-schema checks were reviewed. The Paimon full-schema LTZ condition forces JNI even without an LTZ read, and can reject a metadata-column query (P2).
- Coverage and results: BE, FE, Java, and regression tests plus 44 changed golden files were inspected. Expected types/values align with the new mappings apart from the findings above. No build or test was executed because this review's instructions require static inspection; author/CI claims were not treated as independent validation.
- Observability: The inspected error paths report failures through existing statuses/exceptions; no separate missing log or metric was substantiated.
- Persistence and failover: Catalog marker edit-log writes, replay, and master promotion were traced; no unjournaled state or distinct failover bug was established.
- Writes and atomicity: Iceberg Arrow/partition conversion and JDBC bind/transaction paths were reviewed. The JDBC handler-selection error affects TIMESTAMPTZ write semantics (P1); no separate commit or crash-safety issue was substantiated.
- FE/BE transport: The new Iceberg Thrift field is set correctly in production, while its test fixture is wrong; the JDBC enum reaches Java as the wrong string (both inline). Other changed carriers were traced without a separate issue.
- Performance and other: Unneeded Paimon JNI fallback loses native ORC splitting and parallelism (P2). No further evidenced performance or correctness issue survived the final sweep.
User focus: no additional focus was provided. All four new issues are inline; no existing blocking comment IDs apply.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29173 ms |
Contributor
There was a problem hiding this comment.
Request changes. Static review of PR #68786 at f2c7b1d41ad2ba9d5ab12e2ee3b6859123ba5563 across all 219 changed files. Ten new inline findings: two P1, six P2, and two P3. The P1s are unreadable Fluss binary-partition tables after catalog migration and a MySQL mixed-timestamp predicate that can silently discard rows. The remaining findings cover Iceberg UUID partition reads, migration lock scope, PrestoDB and old Oracle zoned writes, Oracle/Trino year bounds, and two regression suites.
Convergence and prior threads. Three normal full-review passes and separate risk-focused passes covered the changed paths and their upstream/downstream carriers. Round three found three new valuable candidates; the review is incomplete under the review contract's three-round convergence limit, although every currently identified candidate has been verified and accepted or dismissed and every accepted issue is inline below. The four existing inline threads were rechecked at this head: the Iceberg Thrift NULL-key fixture and JDBC terminal-comment/Paimon fallback cases are fixed; the JDBC dialect-name P1 does not apply because the generated Thrift overload returns names and its new test covers the parameter. No existing P0/P1 comment remains applicable, so there are no carried blocker IDs. The related PR #68532 and this PR already acknowledge the mixed-version FE schema-serving window; it is recorded as a known limitation rather than a duplicate inline issue. No additional user focus was provided.
Critical checkpoint conclusions (code-review skill Part 1.3):
- Goal and proof: The change enables default VARBINARY and zoned-timestamp mappings across external catalogs and read/write transports. It adds broad unit, integration and regression cases, but the ten inline gaps mean the goal is not fully met; the tests were inspected, not run here.
- Scope and clarity: The 219-file change is broad but follows the shared mapping rollout. The two new regression blocks violate repository output/cleanup conventions; no unrelated production change was established.
- Concurrency and locks: Master promotion holds the outer
CatalogMgrwrite lock while a deferred authorization-plugin close runs (M-002). No further new lock-order failure was substantiated. Existing connector-close work under the lock also occurs in ordinary ALTER and is a separate pre-existing pattern. - Lifecycle and initialization: CREATE, ALTER, replay, migration, connector reset and promotion were traced. Journal-first migration and retry/checkpoint ordering are coherent; the authorization cleanup lock scope is not. No new cross-translation-unit static initializer dependency was found.
- Configuration: The binary and timestamp mapping markers are compatibility settings forced or normalized to true, rather than dynamically switchable options. That makes Fluss's unsupported binary partition mode unavoidable after migration or ALTER (M-005).
- Compatibility: FE/BE Thrift field 20 and JDBC dialect transport were rechecked; the old fixture and enum-name concerns are resolved. The acknowledged mixed-FE-version schema window remains. The newly supported zoned Oracle write path lacks an
ojdbc6bind (M-011). - Parallel paths: Native/JNI, scalar/array, table/TVF, static/dynamic Iceberg writes, partition constants, NULL, predicates and external sinks were traced. M-001, M-004, M-007, M-008, M-010 and M-011 are the concrete mismatches; related carriers examined did not yield another distinct issue.
- Conditional checks: Paimon historical ORC field-ID routing and binary-expression guards were checked. JDBC's new instant guard misses the MySQL two-column, mixed-type comparison (M-010); the out-of-range Oracle/Trino readers lack the equivalent PostgreSQL bound check (M-007/M-008).
- Test coverage and negative cases: Added cases cover many ordinary bytes, time zones and NULLs, but omit a UUID identity scan, upgraded Fluss binary partitions, official PrestoDB and old Oracle writes, Oracle/Trino out-of-range values, and a non-UTC MySQL two-column comparison. M-003/M-009 also need runner-generated ordered output and no post-test table drop.
- Test results: Golden-output changes were inspected against the code paths; no additional wrong expected row was substantiated. No build, unit test, regression test or live database case was executed in this review, so author/CI claims are not independent validation.
- Observability: Existing catalog/connector error paths and identifiers were inspected; no separate missing log or metric was substantiated. Explicit range rejection would make M-007/M-008 diagnosable instead of packing unsupported years.
- Persistence and transactions: Catalog marker logs are written before local apply, replay reproduces the marker, and crash/retry/checkpoint ordering appears coherent. No new storage visible-version, delete-bitmap or transaction-log inconsistency was found.
- Writes and crashes: Iceberg static/dynamic partition routing, typed commit values and file cleanup were traced without another proven atomicity or crash leak. The concrete new write failures are M-004 and M-011; the lock issue is M-002.
- FE-to-BE values: The new Thrift NULL-key field and JDBC dialect/parameter transport were checked at each sender/reader reached by the diff; no additional missing propagation was found.
- Performance: Paimon schema checks are cached by scan context/schema ID, and no distinct unbounded allocation or hot-loop regression was proven. Slow plugin cleanup under the global catalog lock is the concrete contention issue (M-002).
- Other issues: The remaining speculative or already-reported candidates were dismissed with code evidence or existing review context. The ten comments below are the complete accepted set from this capped static review.
Contributor
TPC-H: Total hot run time: 35956 ms |
Contributor
ClickBench: Total hot run time: 25.44 s |
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Oct 8, 2026
### What problem does this PR solve? Related PR: apache#68786 Problem Summary: Preserve unconditional Fluss binary and timestamp types and encode binary/UUID partition constants for VARBINARY scans. Keep mixed instant and wall-clock predicates local, release the outer catalog migration lock before plugin cleanup, and reject Oracle/Trino UTC years outside the supported range. Use server-side casts for PrestoDB and legacy Oracle zoned writes, and avoid PrestoDB's unsupported typed timestamp getter. Update stale SQL Server unit-test expectations and generate ordered regression outputs through actual execution. ### Release note Fluss BINARY/BYTES always map to VARBINARY, TIMESTAMP_LTZ to TIMESTAMPTZ, and TIMESTAMP to DATETIMEV2. PrestoDB and older Oracle JDBC drivers support zoned timestamp writes through explicit server conversions. ### Check List (For Author) - Test: 282 targeted FE/connector/JDBC tests passed, including real PrestoDB and Oracle 11g round trips in three JVM zones. Both VARBINARY and PostgreSQL regression suites generated their outputs and passed comparison reruns. Full FE Checkstyle and clang-format 16 passed. - Behavior changed: Yes, unconditional Fluss mappings and corrected external partition/time semantics. - Does this need documentation: No, follows the external type mapping behavior documented by the parent change.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review complete at head 542257b6961afcc5d7c93c4bd170dc82f0320a05 after two convergence rounds and a final sweep of all 239 changed paths. Request changes: the new MySQL timestamp predicate case can silently discard rows. The three inline comments cover that case, Iceberg binary partition metadata, and regression test standards. No additional user review focus was supplied. This was static inspection; no build or test was run.
Code-review checkpoints:
- Goal and tests: The change wires binary and zoned timestamp mappings through FE, JNI, BE, catalog migration, and Iceberg writes, with new unit and regression source. The reported JDBC and Iceberg paths prevent complete correctness, and their specific cases lack coverage.
- Scope and clarity: The broad change follows the required cross-layer type contracts. No unrelated production change was substantiated as a separate issue.
- Concurrency and locks: Master promotion journals and applies marker updates under the catalog write lock. Authorization cleanup is deferred until unlock, while connector close can still perform I/O under that lock; this residual is within existing P2 thread 4218883248. No new lock-order or deadlock path was established.
- Lifecycle: Promotion gates serving, replays the journal, migrates markers, then resumes checkpoint/service work. Reset and connector rebuild paths were traced; no distinct retry or startup divergence was found.
- Configuration: CREATE, ALTER, and migration normalize the two mapping markers to true. This is an intentional one-way compatibility migration; no dynamic toggle is promised.
- Compatibility: The static-null Thrift set uses field 20 and FE/BE agree on its representation. Supported rolling upgrades put BE before FE; acknowledged mixed-version limitations remain. No external file rewrite is introduced.
- Parallel paths and conditions: Legacy and plugin JDBC, CDC/TVF exceptions, external connector reads, and Iceberg static/dynamic writes were compared. The JDBC guard handles wall-clock columns but misses literals; Iceberg's partition-key builder still lacks VARBINARY.
- Coverage and results: Relevant tests and generated output changes were inspected, never executed here. New regression checks listed in the P3 comment lack generated output; the reported JDBC literal and Iceberg identity-partition cases are not covered.
- Observability and errors: FE warns when partition-item creation fails but then reports the table unpartitioned, obscuring the loss of pruning. Other error/status and BE null/constant handling examined did not yield a distinct new finding.
- Persistence and failover: Catalog ALTER logging precedes application, and follower replay/retry use the persisted markers. No new EditLog or checkpoint inconsistency was established.
- Writes and crash paths: Iceberg NULL, binary, UUID, timestamp, full-static, hybrid, overwrite, transform, and commit transports were traced. No separate atomicity or crash-leak defect was substantiated.
- FE/BE transport: New binary/instant carriers and the Thrift NULL marker were checked on both sides; the older UUID scan-transport thread is addressed.
- Performance, memory, and other paths: The Iceberg finding disables FE partition pruning. Fluss still loses FE binary partition metadata/pruning, but its earlier P1 listing/SELECT outage is fixed and the narrower residual belongs to existing thread 4218883211. No distinct memory-accounting or further performance issue survived recheck.
Existing P1 threads 4217826031, 4217826038, 4218883211, and 4218883225 were independently checked; none remains applicable at its reported P1 failure mode, so no existing blocker ID is carried.
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29057 ms |
Contributor
TPC-DS: Total hot run time: 151464 ms |
Contributor
ClickBench: Total hot run time: 25.49 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review status: INCOMPLETE under the requested three-round convergence rule: the final risk pass found a new, independently validated Hive ORC V1 issue (M-9), so another no-new-findings round could not be run. All 265 changed paths were covered in each normal pass, every raised candidate has a disposition, and all eight accepted findings are inline below. Two are P1 (Iceberg partition/file identity and official PrestoDB zoned-array scans); six are P2 (four external JNI timestamp range paths, nested MaxCompute writes, and Hive ORC V1 range packing). I recommend changes before merge.
Existing P1 disposition: no previously reported P1 still applies with its original scope at this head, so existing_blocking_comment_ids is empty. The BE Thrift test uses field 20; the JDBC dialect name resolves the generated Thrift to_string overload; upgraded Fluss nodes decode binary partition names (the conditional old-follower failover case is deduplicated against that thread at lower severity); and both JDBC mixed-timezone predicate forms are held local by hasWallClockValue with remote LIMIT suppressed when a filter is dropped. Existing P2/P3 inline threads remain duplicate fences and were not reposted. The user focus file listed no additional focus.
Code-review critical checkpoints:
- Goal and tests: the change aims to preserve external binary bytes and zoned instants across read/write paths. Ordinary, NULL, DST, pre-epoch, partition, and nested fixtures were added, but the eight findings show the goal is incomplete and their boundary/mixed-value cases lack coverage.
- Scope: the 265-path change is broad but centered on type mapping, transport, catalog migration, and tests; no unrelated production edit was identified.
- Concurrency: catalog migration journals/applies under the global catalog write lock and performs detached cleanup after unlock; writer caches are per sink instance. The Iceberg path-key collision is a correctness fault, not a lock race; no separate lock-order or shared-state defect survived.
- Lifecycle/statics: connector reset/close and master-promotion sequencing were traced; function-local epoch statics have ordered initialization. No new cross-translation-unit initializer dependency or unreleased lifecycle was found.
- Configuration: legacy binary/instant mapping flags now normalize to true on CREATE/ALTER and replay; no new process setting requiring dynamic propagation was added. The existing force_jni_scanner and enable_file_scanner_v2 switches were checked for reachable paths.
- Compatibility: optional Thrift field 20 agrees across FE, BE, and the fixed fixture. The old-follower Fluss case is already in an existing thread; no other new rolling-format issue was substantiated.
- Parallel paths: legacy/plugin JDBC, native/JNI, Parquet/ORC, scalar/nested, TVF/table, and read/write counterparts were compared. Source-specific gaps are recorded separately in the inline findings.
- Conditions: JDBC wall-clock predicate guards and dropped-filter LIMIT handling, Paimon historical field-ID fallback, and static/hybrid NULL branches were traced; no additional incorrect guard survived.
- Test coverage: changed unit/integration and regression suites exercise common and selected negative cases, but miss the accepted official-driver, year-10000, mixed Iceberg binary/NULL, and nested MaxCompute cases.
- Expected results: changed .out fixtures were compared with ordered queries by static inspection; their execution and generation were not independently verified. Existing style comments cover fixed assertions and post-run drops.
- Observability: ordinary connector/BE errors retain identifying context, while the silent invalid timestamp packing and mislabeled Iceberg file are the issues reported here; no distinct metrics/logging gap was proven.
- Persistence/failover: marker migration journals before local apply and replay restores it after a crash; the residual mixed-version Fluss concern was deduplicated with the prior thread.
- Writes/atomicity: Iceberg can commit a file with rows from two logical partitions under one first-row partition value; nested MaxCompute zoned writes fail before completion. Other inspected write and cleanup paths showed no separate atomicity defect.
- FE/BE variables: the mapping markers and optional NULL-key Thrift field are passed to the relevant readers/writers; typed partition value and JNI timestamp transport were traced end to end.
- Performance/memory: projected/historical Paimon checks and connector conversions were inspected for repeated work and allocation; no distinct material regression was substantiated.
- Other issues: the final changed-file/accepted-anchor sweep found no unresolved candidate, while the final-round new issue leaves convergence incomplete as stated above.
Validation was static only as required: no build, test, or source edit was performed. The read-only BE header-hygiene gates passed; inspected test files and author/CI claims are not independent execution evidence.
morningman
added a commit
to morningman/doris
that referenced
this pull request
Oct 13, 2026
…Z binding only ### What problem does this PR solve? Issue Number: None Related PR: apache#65868, apache#68786 Problem Summary: apache#68786 binds every Paimon TIMESTAMP WITH LOCAL TIME ZONE column as TIMESTAMPTZ; enable.mapping.timestamp_tz no longer selects a DATETIMEV2 binding. The cast value of a static LTZ partition is therefore always the instant in UTC with its offset, and the session-zone path of PaimonWriteBinding, with its check for a session time inside a DST gap, can no longer run. That check existed because the BE turned a DATETIMEV2 row value into an instant with cctz while the FE used java.time; a TIMESTAMPTZ row value and the static value now come from the same FE cast. Remove the path and the session zone it needed, and keep the rejection of an instant whose local time the FE JVM zone repeats. PaimonConnectorTransactionTest, added by apache#68858 after this change was written, builds the write binding through the new create signature. ### Release note None ### Check List (For Author) - Test: Unit Test - PaimonWriteBindingTest: the LTZ zone move, the overlap rejection and the round trip through Paimon's parser, all for TIMESTAMPTZ values. - The Paimon connector (701) and fe-connector-spi (147) tests, and the fe-core binding, BindSink and insert command tests (52) pass. - Behavior changed: No (the removed path had become unreachable) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
morningman
added a commit
to morningman/doris
that referenced
this pull request
Oct 13, 2026
…Z binding only ### What problem does this PR solve? Issue Number: None Related PR: apache#65868, apache#68786 Problem Summary: apache#68786 binds every Paimon TIMESTAMP WITH LOCAL TIME ZONE column as TIMESTAMPTZ; enable.mapping.timestamp_tz no longer selects a DATETIMEV2 binding. The cast value of a static LTZ partition is therefore always the instant in UTC with its offset, and the session-zone path of PaimonWriteBinding, with its check for a session time inside a DST gap, can no longer run. That check existed because the BE turned a DATETIMEV2 row value into an instant with cctz while the FE used java.time; a TIMESTAMPTZ row value and the static value now come from the same FE cast. Remove the path and the session zone it needed, and keep the rejection of an instant whose local time the FE JVM zone repeats. PaimonConnectorTransactionTest, added by apache#68858 after this change was written, builds the write binding through the new create signature. ### Release note None ### Check List (For Author) - Test: Unit Test - PaimonWriteBindingTest: the LTZ zone move, the overlap rejection and the round trip through Paimon's parser, all for TIMESTAMPTZ values. - The Paimon connector (701) and fe-connector-spi (147) tests, and the fe-core binding, BindSink and insert command tests (52) pass. - Behavior changed: No (the removed path had become unreachable) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Oct 13, 2026
### What problem does this PR solve? Issue Number: None Related PR: #62183, #55651, #68786, #56181 Problem Summary: **In short.** A JDBC catalog that points at another Doris shows the remote VARBINARY columns as STRING (`text`). This PR maps them to VARBINARY again, as the fe-core JDBC client and branch-4.x do; a VARBINARY element of an array stays STRING, because the scanner reads array elements only as text. Reading a VARBINARY column through a Java scanner, which the new mapping now does for a remote Doris, has leaked a native buffer on every batch since VARBINARY was added to the JNI vectors, so this PR frees it too. It is not a port of a branch-4.x change; it came up while comparing the binary type mappings of the two branches. **Background.** When the JDBC connector became a plugin (#62183), the Doris dialect of `JdbcMySQLConnectorClient` mapped a remote `VARBINARY` to `STRING`. The fe-core `JdbcMySQLClient` it replaced, and branch-4.x, map it to `VARBINARY` with the column size the remote reports (#55651). For every other JDBC source, binary types already map to `VARBINARY` regardless of the deprecated `enable.mapping.*` flags (#68786, pinned by `JdbcLogicalTypeMappingTest`). A remote Doris exposes VARBINARY through views and external tables. A remote Doris returns an `ARRAY` column as text, which spells a VARBINARY element as `0x` hex (`[0x616263, null, 0x]`), and `MySQLTypeHandler` decodes the elements from that text: into strings, numbers and dates, not into bytes. On the BE side, a Java scanner hands each batch over in a `VectorTable`. A VARBINARY column of that table keeps a 16-byte `[length, address]` record per row in an off-heap buffer of its own, and each record points at a separate block with the row's bytes. **The problem, and what it cost.** | A JDBC catalog connected to the local Doris | master | with this PR | |---|---|---| | `DESC` of a view with `cast('abc' as varbinary(16))` | `b text` | `b varbinary(65535)` (the length the remote reports through the JDBC metadata) | | `SELECT b` | the value as a string | `0x616263`, the remote bytes; `hex(b)` = `616263` | | a view with `array(cast('abc' as varbinary(16)), cast(null as varbinary(16)), cast('' as varbinary(16)), cast(unhex('00ff') as varbinary(16)))` | `array<text>`, `["0x616263", null, "0x", "0x00FF"]` | the same | Mapping the array element to VARBINARY as well, as branch-4.x does, makes the column `array<varbinary(2147483647)>`, and reading it fails with `Unsupported array child type: VARBINARY`. `VectorColumn.close()` freed the row blocks of a VARBINARY column but never its record buffer (16 bytes per row of batch capacity, about 64 KB for a batch of 4064 rows). BE closes the Java table after every batch (the V1 reader through `releaseTable`, the V2 reader through `releaseColumn` per column), and a Java UDF closes its output table after every call, so every JNI read of a binary column (the binary types of every JDBC source, Paimon, Hudi, MaxCompute) and every Java UDF that returns VARBINARY lost one such buffer per VARBINARY column per batch. The allocator behind it is plain malloc / free, so nothing reclaims the buffer. **How this PR fixes it.** - The Doris dialect maps `VARBINARY` to `VARBINARY`, taking the length from the type name when it carries one and otherwise from the column size, like the dialect's `VARCHAR` branch and the fe-core client. - A VARBINARY element of a remote Doris array, nested or not, keeps mapping to `STRING`. - `VectorColumn.close()` frees the record buffer of a VARBINARY column after the row blocks it points to. **Results.** - FE UT: `JdbcLogicalTypeMappingTest#remoteDorisVarbinaryKeepsBytesAndDeclaredLength` fails without the mapping (STRING) and passes with it; `#remoteDorisVarbinaryArrayElementsStayString` (`ARRAY<VARBINARY>`, `ARRAY<VARBINARY(16)>`, `ARRAY<ARRAY<VARBINARY(16)>>`) fails when the array element follows the scalar mapping. All fe-connector-jdbc tests (251) pass. - jni-spi UT: `VectorTableVarbinaryCloseTest#closeFreesTheRowRecordsOfAVarbinaryColumn` closes a table with a VARBINARY column, as a scanner does after each batch, and checks that the record buffer is freed; it fails without the fix. All jni-spi tests (24) pass. jni-spi gains `mockito-inline` in test scope to observe the static `OffHeap` calls. - Regression: `test_doris_jdbc_catalog_varbinary` connects a JDBC catalog to the local Doris and reads a view with a VARBINARY column and a view with a VARBINARY array. With the plugin from master the scalar column is described as `text` and the suite fails; with the scalar mapping alone the array column is described as `array<varbinary(2147483647)>` and the suite fails; with this PR it passes.
morningman
added a commit
to morningman/doris
that referenced
this pull request
Oct 13, 2026
…Z binding only ### What problem does this PR solve? Issue Number: None Related PR: apache#65868, apache#68786 Problem Summary: apache#68786 binds every Paimon TIMESTAMP WITH LOCAL TIME ZONE column as TIMESTAMPTZ; enable.mapping.timestamp_tz no longer selects a DATETIMEV2 binding. The cast value of a static LTZ partition is therefore always the instant in UTC with its offset, and the session-zone path of PaimonWriteBinding, with its check for a session time inside a DST gap, can no longer run. That check existed because the BE turned a DATETIMEV2 row value into an instant with cctz while the FE used java.time; a TIMESTAMPTZ row value and the static value now come from the same FE cast. Remove the path and the session zone it needed, and keep the rejection of an instant whose local time the FE JVM zone repeats. PaimonConnectorTransactionTest, added by apache#68858 after this change was written, builds the write binding through the new create signature. ### Release note None ### Check List (For Author) - Test: Unit Test - PaimonWriteBindingTest: the LTZ zone move, the overlap rejection and the round trip through Paimon's parser, all for TIMESTAMPTZ values. - The Paimon connector (701) and fe-connector-spi (147) tests, and the fe-core binding, BindSink and insert command tests (52) pass. - Behavior changed: No (the removed path had become unreachable) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
924060929
pushed a commit
that referenced
this pull request
Oct 14, 2026
…anges (#69001) ### What problem does this PR solve? Issue Number: None Related PR: #66012, #62821, #68124, #68786 Problem Summary: **In short.** This PR ports to master the tests of branch-4.1 #66012 whose behavior master already has, so that behavior is pinned. It changes no production code. #66012 made external scans and file loads send each file's partition values with explicit NULL flags; on master the same behavior arrived through #62821 and the connector plugins, but nothing tests three parts of it. **Background.** A file scan range tells BE the partition values of its file in `columns_from_path_keys`, `columns_from_path` and `columns_from_path_is_null`, and BE fills the partition columns from them instead of reading them from the file. Two producers matter here: - `FilePartitionUtils.parseColumnsFromPathWithNullInfo` parses `key=value` directories for Hive-style tables, TVFs and file loads. The Hive default partition (`__HIVE_DEFAULT_PARTITION__`) becomes SQL NULL through the null flag, and a literal `\N` stays data. - The Iceberg connector sends a file's identity partition values from the file's own partition spec and partition data, not from its path. After partition evolution, files of different specs carry different keys. **The problem, and what it cost.** These are the parts of #66012's tests that hold on master and that no master test covers. Each of them decides what BE writes into a partition column: | Behavior | What a regression would do | |---|---| | A case-insensitive parse lowercases a copy of the keys, not the caller's list | The caller's column list changes under it | | `__HIVE_DEFAULT_PARTITION__` → `""` with the null flag; a literal `\N` → `\N` without it; an ACID layout skips its delta directory | A NULL partition reads as an empty string, or a `\N` value reads as NULL | | An Iceberg range sends its identity values in spec order, a NULL value through the null flag | Values land in the wrong columns, or a NULL partition reads as an empty string | | After the spec evolves from (Region, Dt) to (Dt, Category), a file of each spec sends its own spec's keys | Files of one spec get the other spec's columns, which BE then fills as NULL | **How this PR fixes it.** It adds three tests: - `BrokerUtilTest#parseColumnsFromPathPreservesNullMetadataAndInputKeys` (#66012's test of the same name, calling `FilePartitionUtils`). - `IcebergScanPlanProviderTest#planScanSendsIdentityPartitionsInSpecOrderWithNullFlags` (from #66012's `IcebergScanNodeTest#testSetPartitionValuesBuildsStableAlignedMetadata`). - `IcebergScanPlanProviderTest#planScanSendsEachFilesOwnIdentityPartitionsAfterSpecEvolution` (from `#testSetPartitionValuesUsesPerSpecMetadataWithFileScannerV2`; it also checks the old-spec file). The Iceberg tests plan a scan of an in-memory table, as the neighboring #68124 test does. #66012 changes six more tests that are not ported, because master differs on purpose: | branch-4.x test | Why it does not apply to master | |---|---| | `BrokerUtilTest#normalizeColumnsFromPathPreservesNullInfo` | The connectors map the Hive default partition to NULL themselves (`HiveScanRangePartitionValuesTest`), so fe-core's normalization only maps a Java null | | `IcebergUtilsTest#testGetCommonIdentityPartitionColumnsUsesSafeIntersection`, `IcebergScanNodeTest#testSetPartitionValuesKeepsCommonMetadataWithLegacyFileScanner` | branch-4.x keeps only the identity columns every spec shares; master keeps the union, so a file written before the spec evolved to unpartitioned keeps its values (#68124) | | `IcebergUtilsTest#testMappedTypesAreExcludedFromPartitionMetadata`, `IcebergScanNodeTest#testSetPartitionValuesSkipsValuesUnsupportedByMappedTypes` | Since #68786, master sends UUID and TIMESTAMPTZ identity values too | | `BrokerUtilTest#parseColumnsFromPathForLoadKeepsLegacyNullSemantics` | A real difference: on branch-4.x a file load stores a literal `\N` path value as NULL, master stores the two characters `\N`. It needs a code change and comes in a separate PR | **Results.** The three tests pass on master without any code change, and each catches a regression of what it pins: | Change to master | Fails | |---|---| | `FilePartitionUtils` stops flagging the Hive default partition as NULL | `parseColumnsFromPathPreservesNullMetadataAndInputKeys` | | `FilePartitionUtils` lowercases the caller's key list in place | `parseColumnsFromPathPreservesNullMetadataAndInputKeys` | | `IcebergScanRange` sends a NULL identity value without the null flag | both new Iceberg tests | | The Iceberg planner sends every identity key of every spec for each file | `planScanSendsEachFilesOwnIdentityPartitionsAfterSpecEvolution` (and the existing `planScanKeepsUnpartitionedSpecIdentityAfterEvolvingToPartitioned`) | Original author: @suxiaogang223 --------- Co-authored-by: Socrates <suyiteng@selectdb.com>
morningman
added a commit
that referenced
this pull request
Oct 14, 2026
…types (#68874) ### What problem does this PR solve? Issue Number: None Related PR: #65868 Problem Summary: **In short.** This ports the static-partition part of branch-4.1 #65868 to the Paimon connector. An `INSERT OVERWRITE ... PARTITION (...)` named the Paimon partition by the literal as the user wrote it, while the rows carry that literal cast to the column type. Overwrites whose literal is not spelled the column type's way failed, and overwrites of a TIMESTAMP WITH LOCAL TIME ZONE partition failed whenever the session zone differed from the FE JVM zone; when no rows were written to trip Paimon's check, the overwrite silently deleted the wrong partition. Connectors now get the cast values through a new write-handle getter, and Paimon uses them for an overwrite. **Background.** On a Paimon table, the bind step materializes the literal of `PARTITION (col = <literal>)` into every row with `castIfNotSameType`, so the rows carry the value in the column's type. The static partition spec handed to connectors is built separately: `PluginDrivenInsertCommandContext` keeps `Literal.getStringValue()` of the literal as written, and `PaimonWriteBinding` passed that string to Paimon's `withOverwrite`. Paimon parses the string with the column type to find the partition to replace, parses an LTZ value as local time in the FE JVM zone, and reads the table's `partition.default-name` as the NULL partition. branch-4.1 #65868 casts each literal to its column type first, moves an LTZ value from the session zone to the JVM zone, and rejects a value equal to `partition.default-name`; master has none of this. **The problem, and what it cost.** On a local FE + BE with a filesystem Paimon catalog, session zone Asia/Shanghai, FE JVM zone Europe/London: | Statement | master | |---|---| | `INSERT OVERWRITE t PARTITION (p = true) VALUES (...)`, `p INT` | fails: `NumberFormatException: For input string: "true"` | | `... PARTITION (dt = 20240101) ...`, `dt DATE` | fails: `the changes in dt=19723/ does not belong to this partition` (Paimon reads 20240101 as a day number) | | `... PARTITION (ts = '2024-01-15 08:30:45') ...`, LTZ `ts` | fails: `the changes in ts=2024-01-15T00%3A30%3A45/ does not belong to this partition` | | the same LTZ overwrite with no rows (`SELECT ... LIMIT 0`) | succeeds, deletes the partition `2024-01-15 16:30:45` (session time) and keeps `08:30:45` | | an overwrite with no rows of `region = '<partition.default-name>'` | succeeds, deletes the NULL partition | The two empty overwrites are silent data loss: the statement reports success and the deleted partition is not the one the user named. These were measured before #68786, which now binds every Paimon LTZ column as TIMESTAMPTZ; it leaves the static partition spec as written, so the LTZ rows still apply. **How this PR fixes it.** - `ConnectorWriteHandle#getCastStaticPartitionSpec` (new): the static partition values cast to the types of their columns, keyed like `getStaticPartitionSpec`, which still carries the literals as written. SQL NULL keys stay in `getStaticPartitionNullKeys` only; a key that names no column (an Iceberg partition field) keeps its literal. The default, for a handle that does not cast, is the spec as written without its null keys. - fe-core: `InsertIntoTableCommand` records the sink's bound target schema on `PluginDrivenInsertCommandContext`, and the write handle computes the cast values on demand with `checkedCastTo`, the cast constant folding applies to the row value. A string written into a string column follows BindSink instead, which does not cast it: the value is cut to a CHAR / VARCHAR length in code points when the session truncates strings on insert (`BindSink.truncatesStringOnInsert()`, shared with BindSink), and kept as written otherwise. Only a connector that asks pays for the cast and can fail on it, so Iceberg, MaxCompute and Hive, which keep reading the spec as written, see no change. A value that does not fit its column type fails the cast instead of becoming NULL, as on branch-4.x. A binary value is spelled `0x` and its bytes in hex, as in the spec as written. - Paimon: `PaimonWriteBinding` resolves the static partition of an overwrite from the cast values. An INSERT writes each row to the partition its own values name and never reads the static partition, so Paimon asks for the cast values only for an overwrite, and an INSERT is planned as before (branch-4.x casts for an INSERT too). Doris binds an LTZ column as TIMESTAMPTZ (#68786), so the cast value of an LTZ partition is the instant in UTC with its offset; it is moved to the FE JVM zone and written with a space between date and time, which Paimon's parser needs. An overwrite value equal to `partition.default-name` is rejected. An overwrite also rejects an LTZ value whose local time in the JVM zone falls in a DST overlap, since both instants format to the same value and Paimon reads it as the earlier one (as branch-4.2 does, #68532). A static overwrite that gives a binary partition column a value other than NULL is rejected: Paimon reads a static value as text and takes its UTF-8 bytes, so neither the cast value nor the `0x4142` that master passes names the partition, and an overwrite with no rows committed against another one. **Results.** The table above, with this PR: | Statement | this PR | |---|---| | `PARTITION (p = true)` | replaces `p = 1` | | `PARTITION (dt = 20240101)` | replaces `dt = 2024-01-01` | | LTZ overwrite | replaces the named partition | | LTZ overwrite with no rows | deletes the named partition only | | `region = '<partition.default-name>'`, with or without rows | fails: `... cannot be represented in a static overwrite`, no data changes | | `INSERT OVERWRITE ... PARTITION (dt = '2024-13-45')` | fails: `date/datetime literal [2024-13-45T00:00:00] is invalid` | | `INSERT INTO ... PARTITION (dt = '2024-13-45')` | unchanged: writes `dt = NULL` | | `PARTITION (p = 'abcd')` on a CHAR(2) column, INSERT or OVERWRITE | writes and replaces `p = 'ab'`, where the rows go | | overwrite with no rows of `p = '😀x'` on a VARCHAR(1) column | deletes the partition `😀` only | | LTZ overwrite whose value is repeated in the FE JVM zone (UTC session, `2024-10-27 01:30:45` with the JVM in Europe/London) | fails: `... is ambiguous in FE JVM time zone Europe/London ...`, no data changes | | LTZ overwrite of a value skipped in the session zone (`2024-03-31 01:30:45` in a Europe/London session) | replaces the partition its rows are written to, the instant of the DST change (01:00:00Z); an empty one deletes that partition | | `PARTITION (p = X'4142')` on a BYTES column, with or without rows | fails: `Static overwrite of binary partition column 'p' is not supported`, no data changes (before, an empty one committed and left the partition `AB` in place) | Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 15, 2026
morningman
added a commit
to morningman/doris
that referenced
this pull request
Oct 16, 2026
… catalogs ### What problem does this PR solve? Issue Number: None Related PR: apache#68786 Problem Summary: apache#68786 made `OracleTypeHandler.initializeStatement` set the session time zone of every borrowed connection to UTC through Oracle's driver (`oracle.jdbc.OracleConnection#setSessionTimeZone`, loaded reflectively from the driver's class loader), so that the driver reads TIMESTAMP WITH LOCAL TIME ZONE as an instant. `JdbcTypeHandlerFactory` gives the same handler to an Oracle-mode OceanBase catalog (`OCEANBASE_ORACLE`), which connects through OceanBase's driver. Its class loader has no `oracle.jdbc` classes, so every scan of such a catalog failed before its query was prepared, with "Failed to initialize Oracle session time zone" caused by a ClassNotFoundException, whatever columns it read. Writes were not affected. The handler now takes the table type, as `MySQLTypeHandler` does, and sets the Oracle session time zone only for an Oracle catalog. An Oracle-mode OceanBase catalog prepares its statements as it did before apache#68786, and reads TIMESTAMP WITH LOCAL TIME ZONE as OceanBase's driver returns it. ### Release note Fix scans of Oracle-mode OceanBase JDBC catalogs failing with "Failed to initialize Oracle session time zone". ### Check List (For Author) - Test: Unit Test (OracleSessionTimeZoneTest; all jdbc-scanner tests). No end-to-end test: OceanBase's Oracle mode is only in its enterprise edition, and the regression environment runs OceanBase CE in MySQL mode. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
morningman
added a commit
that referenced
this pull request
Oct 16, 2026
…69142) ### What problem does this PR solve? Issue Number: None Related PR: #68921 Problem Summary: **In short.** `ColumnVarbinary` has no CRC32 / CRC32C hash, so every exchange that partitions rows by a VARBINARY value fails. Since #68786 reads Iceberg BINARY and FIXED as VARBINARY, MERGE INTO an Iceberg table with an `identity` or `truncate` partition on a binary column fails, and #68921 has to leave INSERT into such a table on random writes. This PR hashes VARBINARY values by their bytes, exactly as `ColumnString` does, and lets the Iceberg connector route binary partition values on backends that can hash them. **Background.** An exchange that partitions rows hashes the partition key of each row with CRC32 (`update_crcs_with_value`, `update_crc_with_value`) or CRC32C (`update_crc32c_batch`, `update_crc32c_single`): a shuffle join on its keys, an Iceberg INSERT routed by the transformed partition value (#68921), and MERGE INTO an Iceberg table, which routes the rows it writes through the same Iceberg partition function. Array, map and struct keys hash their elements through the same methods. `ColumnVarbinary` inherits these methods from `IColumn`, where they throw `NOT_IMPLEMENTED_ERROR`. **The problem, and what it cost.** On a local single-BE cluster with an Iceberg REST catalog (tables created by Spark, `parallel_pipeline_task_num = 8`): | Statement | master | with this PR | |---|---|---| | MERGE INTO a table partitioned by `identity(binary_key)` or `truncate(1, binary_key)`, with a `numbers()` source | fails: `update_crc32c_batch is not supported for ColumnVarbinary` | succeeds; the rows match what Spark reads back | | INSERT OVERWRITE of 10000 rows into four `identity(binary_key)` partitions from a `numbers()` source | random writes: 8 data files (measured with this PR at `be_exec_version` 18, the plan master makes) | routed: 4 data files, one per partition | | `SELECT ... JOIN [shuffle] ... ON l.a = r.a` on `ARRAY<VARBINARY>` keys | fails: `update_crc32c_single is not supported for ColumnVarbinary` | same rows as `JOIN [broadcast]` | A scalar VARBINARY comparison is rejected during analysis, so a scalar VARBINARY never becomes a join key; the shuffle join only reaches the exchange through a nested type. **How this PR fixes it.** - **BE.** `ColumnVarbinary` implements the four CRC methods by hashing each value's bytes, with and without a null map, as `ColumnString` does, so the same bytes get the same hash in both columns. SipHash and xxHash (`update_hash_with_value`, `update_hashes_with_value`, `update_xxHash_with_value`) stay unsupported, and so does tablet routing of VARBINARY. - **Mixed versions.** BE exec version 19 marks the capability and FE `max_be_exec_version` moves to 19. `IcebergWritePlanProvider.getWriteDistribution` routes a BINARY or FIXED transformed partition value only when the statement's exec version is at least 19; below it the INSERT keeps random writes, as on master today. `bucket(n, binary_key)` yields an INT and is routed from version 18 as before. MERGE needs no FE change. **Results.** - **BE UT.** `ColumnVarbinaryTest.ShuffleHashesMatchStringOfTheSameBytes` checks the four methods against `ColumnString` on empty, NUL, invalid UTF-8 and inline / out-of-line values, with and without a null map and on a sub-range; `ArrayOfVarbinaryHashesLikeArrayOfString` does the same through `ColumnArray` with a NULL element; `ValueHashesAreNotSupported` keeps SipHash and xxHash unsupported. `ExternalTableSinkBinaryHashPartitionerTest.IcebergTransformHashesBinaryPartitionValues` routes `identity` and `truncate[1]` binary values with CRC32 and CRC32C, so equal partition values share a channel. On master the first two and the partitioner case fail with `NOT_IMPLEMENTED_ERROR`, since the methods are inherited from `IColumn`. The three suites (36 cases) pass under ASAN; clang-tidy reports nothing on the changed files. - **FE UT.** `IcebergWritePlanProviderTest#getWriteDistributionRoutesBinaryPartitionValuesOnlyOnBackendsThatHashThem`: identity / truncate on BINARY, identity on FIXED and a mixed INT + BINARY spec keep random writes at exec version 18 and are routed at 19; `bucket(16, binary_key)` is routed at 18. All 1568 tests of fe-connector-iceberg pass. - **Regression.** `test_iceberg_write_binary_partitions` now checks the data files of the multi-writer INSERT and INSERT OVERWRITE (4 for identity, 1 for truncate(1), which folds the keys into one partition) and adds MERGE INTO identity / truncate binary partitions, compared with Spark. `test_varbinary_sql_support` adds a broadcast and a shuffle join on `ARRAY<VARBINARY>` keys with `parallel_pipeline_task_num = 4`; both return the same 12 rows, and the shuffle plan hash-partitions both sides by the array. Both suites pass on a local single-BE cluster against the Iceberg REST docker environment, where their `.out` files were generated; only the new blocks were added. - **Mixed versions.** On the same cluster, with `be_exec_version` set to 18 the INSERT OVERWRITE above writes 8 data files (random writes), and with 19 it writes 4.
3 of 16 tasks
Gabriel39
added a commit
that referenced
this pull request
Oct 19, 2026
…69171) ## Problem and behavior `branch-4.2` is missing several external type mapping fixes already merged into master. This adapted backport aligns these paths while retaining the branch's existing VARBINARY and TIMESTAMPTZ support: | Path | Result | Upstream | | --- | --- | --- | | Parquet/ORC file TVFs | Unannotated binary fields infer VARBINARY with the legacy option absent, false, or true, including nested fields | #68786 | | Oracle JDBC | BINARY_FLOAT maps to FLOAT; BINARY_DOUBLE maps to DOUBLE | #68200 | | SQL Server JDBC | Resolve user-defined aliases by JDBC type code; strip IDENTITY decoration only when the base type agrees with that code | #67916 | | Iceberg metadata scanner | Preserve signed epoch microseconds and timestamp-without-zone wall-clock values through nested structs, arrays, and maps | #68985 | Logical UUID annotations retain their existing file-format semantics instead of being overridden by the raw-binary default. Parquet schema discovery retains canonical STRING values and ORC retains native UUID values. Modern and legacy Parquet ENUM annotations remain STRING. Full external native UUID alignment and nanosecond timestamp support in #69163 remain separate. Golden results in this PR target its own branch-4.2 build and do not assume those reader changes have landed. The Iceberg scanner keeps this branch's full-schema source indexes rather than copying master's projected-row indexing. The master JDBC scanner architecture remains outside this backport. Oracle connections enable binary IEEE floating-point bindings so valid BINARY_DOUBLE values outside NUMBER’s range can be written without overflow. Binary TVF columns display hexadecimal values. Direct CTAS into an internal VARBINARY column is rejected, consistently with master. The CTAS partitioning/distribution suite explicitly decodes its UTF-8 binary fixture into STRING; the dedicated binary suite checks rejection of direct VARBINARY CTAS and byte-preserving HEX materialization. Paimon raw-file TVF expectations now preserve binary bytes even without the obsolete mapping option. Variant shredding assertions likewise require binary metadata and residual values while preserving all physical-layout and logical round-trip checks. ## Validation - Reproduced the Oracle failure against a real Oracle database: the old executor throws `IllegalArgumentException: Overflow` from `setDouble(1e200)`; the fixed executor preserves large/small doubles, floats, and NULLs through INSERT/readback. - Added shared-schema coverage for modern/legacy ENUM annotations and UUID precedence, plus an ENUM-only CTAS regression with both legacy option values. - Updated scalar/nested UUID regression coverage so a raw-binary default cannot override logical UUID annotations. - Built FE and the full ASAN BE from this PR worktree. FE Checkstyle and clang-format 16 passed. - Five Oracle Java tests passed against a real Oracle database, including the optional INSERT/readback integration test; the new Doris JDBC regression block also passed. - Two focused production-library-linked Parquet schema tests passed; the new logical-annotation test failed against the previous implementation as expected. All four modified BE test translation units passed compilation checks. - All seven UUID regression suites and the complete HDFS Parquet group 6 suite passed actual generation followed by independent comparison runs, with zero skipped suites. Updated golden blocks were taken from those executions. The previous coverage build stopped while downloading Maven (`wget` exit 4), before project compilation. This is an infrastructure failure and is retriggered with the updated source. Muted unrelated compaction/cache/S3 timeout cases are not changed here.
morningman
added a commit
that referenced
this pull request
Oct 19, 2026
…69243) ### What problem does this PR solve? Issue Number: None Related PR: #68786 Problem Summary: **In short.** The V1 ORC reader rejects TIMESTAMPTZ values in year 0, which Doris stores and the other read paths return. Found while comparing master with branch-4.x after #68786; the branch-4.x V1 reader has no range check here. **Background.** #68786 reads ORC `TIMESTAMP_INSTANT` as TIMESTAMPTZ and added a UTC range check to the V1 ORC reader before it packs each value, because ORC allows years that Doris cannot hold. Doris TIMESTAMPTZ holds UTC years 0 through 9999: `TimestampTzValue` rejects only years below 0 or above 9999, and a TIMESTAMPTZ column stores `0000-01-01 00:00:00 +00:00`. In the review of #68786 the JNI `VectorColumn` check was changed to reject only years below 0, and master's ORC serde uses the same 0–9999 range. **The problem, and what it cost.** The V1 check kept its lower bound at 0001-01-01T00:00:00Z (`MIN_SECONDS = -62135596800`). An ORC file with a year-0 value fails the scan with `Decoded TIMESTAMPTZ column '...' is outside the Doris 0001-9999 range`, while the V2 reader returns the value. **How this PR fixes it.** The lower bound becomes 0000-01-01T00:00:00Z (`-62167219200`; year 0 is a leap year, 366 days before year 1). The upper bound and the error for values outside the range are unchanged, and the message now says 0000-9999. **Results.** BE unit test on the V1 decode path: | Value (UTC) | Before | After | |---|---|---| | 0000-01-01T00:00:00Z | DataQualityError | Decoded as year 0 | | 0000-12-31T23:59:59Z | DataQualityError | Decoded as year 0 | | 0001-01-01T00:00:00Z, 9999-12-31T23:59:59.999999Z | Decoded | Same | | One second before year 0, 10000-01-01T00:00:00Z | DataQualityError | Same |
morningman
added a commit
that referenced
this pull request
Oct 19, 2026
… catalogs (#69121) ### What problem does this PR solve? Issue Number: None Related PR: #68786 Problem Summary: **In short.** Since #68786, every scan of a JDBC catalog that connects to OceanBase in Oracle mode fails with `Failed to initialize Oracle session time zone`, whatever columns it reads. The Oracle type handler, which such a catalog shares with Oracle catalogs, sets the session time zone through Oracle's JDBC driver, and an OceanBase catalog has OceanBase's driver instead. This PR makes only Oracle catalogs do that. **Background.** BE's JDBC scanner picks a type handler by the catalog's table type (`JdbcTypeHandlerFactory`): `ORACLE` and `OCEANBASE_ORACLE`, which FE reports for an OceanBase database whose `ob_compatibility_mode()` is ORACLE, both get `OracleTypeHandler`. #68786 made its `initializeStatement` set the session time zone of every borrowed connection to UTC through `oracle.jdbc.OracleConnection#setSessionTimeZone`, loaded reflectively from the class loader of the driver's connection, so that Oracle's driver reads TIMESTAMP WITH LOCAL TIME ZONE as an instant: `ALTER SESSION` alone leaves the driver's own zone unset. The review bot raised this for OceanBase on #68786 (thread 4237173029); the thread was still open when it merged. **The problem, and what it cost.** An Oracle-mode OceanBase catalog connects through OceanBase's driver, and the class loader of a catalog's driver only sees that driver. `Class.forName("oracle.jdbc.OracleConnection", ...)` therefore throws ClassNotFoundException, and `initializeStatement` turns it into `SQLException: Failed to initialize Oracle session time zone` before the query is prepared. Every SELECT on such a catalog failed, including ones that read no timestamp column. Writes were not affected, since they do not go through `initializeStatement`. **How this PR fixes it.** `OracleTypeHandler` takes the table type, as `MySQLTypeHandler` does for `MYSQL` and `OCEANBASE`, and sets the Oracle session time zone only for `ORACLE`. An Oracle-mode OceanBase catalog prepares its statements as it did before #68786, and reads TIMESTAMP WITH LOCAL TIME ZONE as OceanBase's driver returns it. The no-argument constructor keeps the Oracle behavior. **Results.** - **FE UT** (`be-java-extensions/jdbc-scanner`). `OracleSessionTimeZoneTest.oceanBaseOracleModeScansWithoutTheOracleDriver` prepares a statement through the factory's `OCEANBASE_ORACLE` handler on a connection defined by the platform class loader, which cannot see `oracle.jdbc` either; before this change it failed with `Failed to initialize Oracle session time zone`, and setting the Oracle session time zone for `OCEANBASE_ORACLE` too turns it red again. `oracleCatalogSetsTheSessionTimeZone` checks that the factory's `ORACLE` handler still sets it. All 57 tests of jdbc-scanner pass (18 skipped: the two integration test classes need their databases). - **End to end.** Not tested: OceanBase's Oracle mode is only in its enterprise edition, and the regression environment runs OceanBase CE in MySQL mode. branch-4.1 and branch-4.2 carry the same session time zone setup in `OracleJdbcExecutor` since #68873, and also give it to `OCEANBASE_ORACLE`.
Merged
4 of 14 tasks
morningman
added a commit
that referenced
this pull request
Oct 21, 2026
…umns as VARBINARY (#69285) ### What problem does this PR solve? Issue Number: None Related PR: #66032, #68786 Problem Summary: **In short.** This ports to master the rule branch-4.1 got in #66032 (`IcebergMetadataOps.resolvePrimitiveTypeForModify`): when `ALTER TABLE ... MODIFY COLUMN` restates the type an Iceberg column is read as, the Iceberg type stays as it is. On master, every MODIFY COLUMN of an Iceberg BINARY or FIXED column, or of such a STRUCT field, ARRAY element or MAP value, fails with `Unsupported type for Iceberg: VARBINARY`, so their nullability, comment and position cannot be changed at all. After this PR they can. **Background.** Since #68786, Doris always reads an Iceberg BINARY column as `VARBINARY` and a FIXED(n) column as `VARBINARY(n)`. MODIFY COLUMN carries the whole column definition, so a statement that only means to change the nullability, comment or position of such a column restates that type, for example `MODIFY COLUMN payload VARBINARY NULL FIRST`. The nested form (`MODIFY COLUMN s.a ...`, `arr.element`, `m.value`) works the same way. The Iceberg connector turns the requested type into an Iceberg type before it looks at the table (`IcebergSchemaBuilder.buildColumnType`), and then diffs it against the current schema. `VARBINARY` has no Iceberg type there: CREATE TABLE and ADD COLUMN reject it on purpose, as branch-4.x does, because a Doris `VARBINARY(n)` holds up to n bytes and an Iceberg FIXED(n) exactly n. **The problem, and what it cost.** The type build fails before the connector ever looks at the current column, so restating the type fails too: | Statement (current Iceberg type) | master | this PR | |---|---|---| | `MODIFY COLUMN payload VARBINARY NULL FIRST` (BINARY) | `Unsupported type for Iceberg: VARBINARY` | succeeds, stays BINARY | | `MODIFY COLUMN digest VARBINARY(16) NULL COMMENT 'digest'` (required FIXED(16)) | same error | succeeds, stays FIXED(16) | | `MODIFY COLUMN s.a VARBINARY COMMENT 'bytes'` (STRUCT field BINARY) | same error | succeeds, stays BINARY | | `MODIFY COLUMN arr.element VARBINARY(4) NULL` (required FIXED(4) element) | same error | succeeds, stays FIXED(4) | | `MODIFY COLUMN m.value VARBINARY NULL` (required BINARY value) | same error | succeeds, stays BINARY | | `MODIFY COLUMN payload VARBINARY(16)` (BINARY: not how it is read) | `Unsupported type for Iceberg: VARBINARY` | unchanged | So a column that users read and write as VARBINARY could not be moved, commented, or made nullable from Doris; only a comment-only `MODIFY COLUMN ... COMMENT '...'` worked, because it carries no type. On branch-4.1 and branch-4.2, `IcebergMetadataOps.resolvePrimitiveTypeForModify` keeps the current Iceberg type in the first five cases and rejects the last one (`Type varbinary(16) is not supported for Iceberg column payload`). **How this PR fixes it.** When the requested type does not build and it is a `VARBINARY`, `IcebergConnectorMetadata` looks up the current type of the target field (the same best-effort lookup it already did to rewrite the nested error message) and keeps it if that type is read as exactly the requested `VARBINARY`, length included (`IcebergTypeMapping.isReadAs`). The schema update then sees an unchanged type and applies only the nullability, comment and position the statement asks for. This is the same check the Paimon connector already makes in `requestedColumnType`. A `VARBINARY` that restates nothing (another length, a column or field of another type) still fails with `Unsupported type for Iceberg: VARBINARY`, as ADD COLUMN does; any other scalar that does not build still fails without loading the table, and a complex type still gets the nested error message as before. A STRUCT, ARRAY or MAP type that contains `VARBINARY` is rejected by Doris's DDL type check (`STRUCT unsupported sub-type: varbinary(...)`) before it reaches the connector, on branch-4.x as well, so it is unchanged. **Results.** - **Regression test**: new `external_table_p0/iceberg/test_iceberg_modify_binary_column`. The table is created through the REST catalog, since neither Doris nor Spark SQL can declare a FIXED column. The suite runs the five statements above, checks with `DESC` (comments shown) that the types are unchanged while the position and comments changed, checks that the required column, ARRAY element and MAP value reject NULL before and accept it after, and compares the rows with what Spark reads. It fails at its first MODIFY with the connector before this PR and passes with it. Seven related suites pass as well: `test_iceberg_schema_change_complex_types`, `test_iceberg_nested_schema_evolution_ddl`, `test_iceberg_nested_schema_evolution_spark_doris_interop`, `iceberg_schema_change_ddl`, `test_iceberg_varbinary`, `test_iceberg_uuid_binary_compatibility` and `test_iceberg_struct_schema_evolution`. - **FE UT**: new `IcebergConnectorMetadataBinaryModifyTest` drives `IcebergConnectorMetadata` into an in-memory Iceberg catalog. Its four restating cases fail before this PR with `Unsupported type for Iceberg: VARBINARY`; the fifth checks that a VARBINARY that restates nothing still fails and leaves the schema unchanged. Turning off the length comparison, the primitive check, the null check, or reading the current type at the top-level column instead of the nested path each turns a case red. All 1615 tests of fe-connector-iceberg pass. Co-authored-by: Gabriel <liwenqiang@selectdb.com>
3 of 16 tasks
morningman
added a commit
to morningman/doris
that referenced
this pull request
Oct 23, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#66538, apache#68786 Problem Summary: Since apache#68786 the BE reads Iceberg BINARY and FIXED columns as VARBINARY. The file scanner V1 (enable_file_scanner_v2 = false) cannot apply an equality delete whose key is such a column. With one key, create_set(TYPE_VARBINARY) throws NOT_IMPLEMENTED. With several keys, ColumnVarbinary does not implement update_hashes_with_value, so the read fails too. And a delete key read as STRING for a VARBINARY data column, or the other way round, is rejected as a type change. branch-4.1 and branch-4.2 (apache#66538) match string and VARBINARY keys by their bytes. This change does the same: a single byte key goes into a STRING set as bytes, composite byte keys are hashed with xxhash over their bytes and compared as bytes, and STRING and VARBINARY keys count as the same type. Unlike branch-4.2, the byte path also reads const key columns: when the FE does not send Iceberg scan semantics v2, a key column that the data file lacks is filled from Const(NULL), and those rows must match a NULL delete key, not an empty string. ### Release note Fix: With enable_file_scanner_v2 = false, reading an Iceberg table whose equality deletes use a BINARY or FIXED key no longer fails. ### Check List (For Author) - Test: Unit Test - be/test/format/table/equality_delete_test.cpp: the 5 tests of branch-4.2 (single and composite VARBINARY keys, STRING delete keys for VARBINARY data and the reverse, NULL keys) and a composite key whose data column is a const NULL. All 6 fail with the current equality_delete.cpp (one aborts on a bad cast) and pass with this change - IcebergReaderTest, ParquetReadLinesTest, IcebergPositionDeleteSysTable*Test and VIcebergDeleteSinkTest pass - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4 of 16 tasks
morningman
added a commit
that referenced
this pull request
Oct 25, 2026
#69503) ### What problem does this PR solve? Issue Number: None Related PR: #66538, #68786 Problem Summary: **In short.** With the file scanner V1, an Iceberg equality delete whose key is a BINARY or FIXED column fails the read, and in one case aborts the BE. This PR matches string and VARBINARY delete keys by their bytes, as branch-4.1 and branch-4.2 do (#66538). **Background.** Since #68786 the BE reads Iceberg BINARY and FIXED columns as VARBINARY. V1 (`enable_file_scanner_v2 = false`) applies equality deletes in `be/src/format/table/equality_delete.cpp`. For one key column, `SimpleEqualityDelete` builds a `HybridSet` with `create_set` on the key type and probes it with the data column. For several key columns, `MultiEqualityDelete` hashes every row with `update_hashes_with_value` and confirms a hash hit with `compare_at`. Neither handles VARBINARY: `create_set` has no VARBINARY set, and `ColumnVarbinary` does not implement `update_hashes_with_value`. The delete file and the data file can also carry the same key as STRING on one side and VARBINARY on the other. `MultiEqualityDelete` rejects that as a type change, and `SimpleEqualityDelete` probes the set with a column of the wrong class. branch-4.1 and branch-4.2 (#66538) store, hash and compare these keys as bytes. **The problem, and what it cost.** From the new unit tests, run against master's `equality_delete.cpp`: | Equality delete key | Master today (V1) | After this PR | |---|---|---| | one VARBINARY key | `VARBINARY predicates are not supported` | the rows with matching bytes are deleted | | one key, VARBINARY in the delete file and STRING in the data file | `VARBINARY predicates are not supported` | deleted by bytes | | one key, STRING in the delete file and VARBINARY in the data file | the BE aborts: `Bad cast from type:doris::ColumnVarbinary to doris::ColumnStr<unsigned int>` | deleted by bytes | | VARBINARY key with an INT key | `Method update_hashes_with_value is not supported for ColumnVarbinary` | deleted by bytes | | composite key, STRING in the delete file and VARBINARY in the data file | `Not support type change` | deleted by bytes | | composite key whose data column is a const NULL STRING | `Method update_hashes_with_value is not supported for Const(Nullable(String))` | the row matching the NULL delete key is deleted | The const column comes from `IcebergReaderMixin::_create_missing_equality_delete_value`: when the FE does not send Iceberg scan semantics v2 and the data file lacks the key column, the column is filled from `create_column_const(1, Field())`. **How this PR fixes it.** - Keys of a string type or VARBINARY are byte keys. A single byte key goes into a STRING `HybridSet` as bytes, and data rows probe it with their bytes. A byte delete key against a non-byte data column is still rejected as a type change. - For composite keys, a byte key is hashed with xxhash over its bytes (`HashUtil::xxHash64WithSeed`, and `xxHash64NullWithSeed` for NULL) on both the delete side and the data side, and a hash hit compares the bytes. STRING and VARBINARY keys count as the same type. - Unlike branch-4.2, the byte path also reads const key columns, so a const NULL key matches a NULL delete key instead of being read as an empty string. - Non-byte keys keep the current code path. **Results.** - BE unit tests: the new `be/test/format/table/equality_delete_test.cpp` has the 5 tests of branch-4.2 and one for a const NULL key. With master's `equality_delete.cpp` all 6 fail (one aborts the test process); with this PR all 6 pass. The other BE tests around V1 Iceberg deletes pass too: `IcebergReaderTest`, `ParquetReadLinesTest`, `IcebergPositionDeleteSysTableReaderTest`, `IcebergPositionDeleteSysTableV2ReaderTest` and `VIcebergDeleteSinkTest`. - No regression test: Spark SQL cannot write equality delete files, and no docker fixture has an equality delete with a BINARY or FIXED key. `test_iceberg_uuid_binary_compatibility` covers UUID keys, on the V2 scanner only.
morningman
added a commit
that referenced
this pull request
Oct 26, 2026
…ames (#69571) ### What problem does this PR solve? Issue Number: None Related PR: #69567 Problem Summary: **In short.** A query of a Paimon table with a BINARY / VARBINARY partition column now fails with an error naming `'partition.legacy-name'` when it would read a non-NULL binary partition whose files Paimon cannot locate, instead of silently returning only the rows of the other partitions. This is the read-side counterpart of #69567, which rejects writes to such tables. **Background.** Paimon looks for the files of a partition under the directory its partition values name, computed when the split is planned; a writer puts the files under the name it computes when it writes. With `partition.legacy-name = true`, the default, the name of a value is its `toString()`, which for a binary value is the Java array object (`[B@1b6d3586`) rather than its bytes, so every resolution differs. With `partition.legacy-name = false` it is the text of the bytes (`p=AB` for `X'4142'`). The option can be changed on an existing table, which changes how existing partitions are looked up but does not move their files. The BE skips a file of an external table that no longer exists (`ignore_not_found_file_in_external_table`, default true), which is meant for files removed between planning and reading. **The problem, and what it cost.** Measured on master with Paimon 2.1.0, on a `file://` Paimon catalog of a local FE + BE, with tables created and written through the Paimon API: | Table | Paimon API read | Doris before this PR | |---|---|---| | legacy names (default), rows in `X'4142'`, `X'EF'`, NULL | fails: `FileNotFoundException` (`p=%5BB@...`) | `SELECT *` returns only the NULL row; `COUNT(*)`, answered from metadata, returns 3 | | legacy names when `X'4142'` was written, then `'partition.legacy-name' = 'false'` and rows in `X'4344'`, NULL | fails: looks for `p=AB`, the file is under `p=%5BB@...` | returns 2 of 3 rows, the `X'4142'` row is missing | | new names when `X'4142'` and NULL were written, then `'partition.legacy-name' = 'true'` | fails | returns 1 of 2 rows | | legacy names, rows only in the NULL partition | reads | reads correctly | So the query succeeds with a partial result and nothing tells the user. Following the obvious fix, setting `'partition.legacy-name' = 'false'`, does not make the old partitions readable either; it only moves the loss to the files written before the change. **How this PR fixes it.** `PaimonScanPlanProvider.planScanInternal` checks every data split that will read files (after the splits whose `COUNT(*)` is answered from metadata) whose binary partition value is not NULL: - if the split is looked up under legacy names (the options of its table, or of its branch for a fallback read), planning fails; - otherwise, if one of its data files was written under a schema whose options use legacy names (the file's `schemaId`, looked up once per schema and branch, inside the catalog authentication) and is also missing at its planned path (`bucketPath/fileName`, the path the native reader is given), planning fails and names that file. The schema alone does not prove how a file was named: a writer's dynamic options (`spark.paimon.*`, Flink hints, `Table.copy`) keep the schema ID, so a file written with a dynamic `'partition.legacy-name' = 'false'` on a table that keeps the default refers to a schema with legacy names while it sits under the bytes of its value, and it is read once the table uses those names; - and, on a table with `'index-file-in-data-file-dir' = 'true'`, every deletion vector the split carries must exist, whether or not its data file has an external path, or planning fails and names the deletion vector file and its data file. Such a table keeps its deletion vectors with its data files: one written while no external data path is configured records only its file name and is looked up under the directory of its partition, which a legacy name never finds, while its data file keeps its external path, and the BE skips the whole data file when it cannot open the deletion vector. The option is immutable, so every writer used the reading table's value. Deletion vectors under the table's `index/` directory do not depend on partition names and are not checked. A NULL partition is named after `partition.default-name` under both namings, and a data file with an external path is read from the path its metadata records, so neither is rejected; the deletion vectors of a file with an external path are still checked. The system tables read through JNI (`$audit_log`, `$row_tracking`) report the options of the main branch, while a fallback split is planned and read under its own branch, so the check resolves the branch that produced each split from the pinned source of the wrapper, as it does for data tables. `$binlog` exposes its partition columns as arrays and is not checked; Paimon's own reader fails on a file it cannot find. A `scan.file-creation-time-millis` read of a fallback table plans the main branch through its `SnapshotReader`, and its plain splits are judged under the main branch. The error names the table, the partition and the option. The existence checks run inside the catalog authentication, once per path and scan: a table that keeps its index files under `index/` and whose files were written under its current naming makes no extra filesystem calls. **Not covered.** A file written with a dynamic `'partition.legacy-name' = 'true'` (or by a Paimon writer too old to know the option) while its schema says `'false'` sits under a legacy name but is looked up under its bytes, and the BE still skips it, as on master and branch-4.2. Catching it would take an existence call for every file of every non-NULL binary partition on every scan. **Results.** On the same local cluster, after this PR: | Query | Doris after this PR | |---|---| | `SELECT *` of the legacy table (also with `force_jni_scanner = true`) | fails: `Cannot read Paimon table d.api_legacy: the table uses legacy partition names ('partition.legacy-name' = 'true'), under which Paimon names partition p = 0xEF after a Java array object rather than its bytes, so the files of the partition are never found. Paimon cannot read the partition either; ...` | | `SELECT COUNT(*)` of the legacy table | 3, answered from metadata as before | | `SELECT * ... WHERE p IS NULL` of the legacy table | the NULL row; Paimon prunes the other partitions | | `SELECT *` of the table switched to new names | fails: `... data file data-b3abc825-...-0.parquet of partition p = 0x4142 was written while the table used legacy partition names ...`, the file the Paimon API read reports as not found | | `SELECT *` of the table switched back to legacy names | fails: `... the table uses legacy partition names ...` | | `SELECT *` of the legacy table with only NULL partitions, and of tables with new names | all rows, the same as the Paimon API read | `PaimonScanPlanProviderTest` adds eight tests: a scan under legacy names fails while the NULL partition and `COUNT(*)` still plan; a file written under legacy names fails after the option changes to false, and a file written under the new names fails after it changes back to true; a file at an external path still plans and points at an existing file; a file at an external path whose deletion vector was written after the external paths were removed (persisted the way `PaimonSparkWriter.persistDeletionVectors` does it for a DELETE) fails and names the deletion vector, while a deletion vector written with the external paths in place still plans and points at an existing file; a file written through `table.copy({"partition.legacy-name": "false"})` on a table that keeps the default fails as a legacy lookup, then plans once the option is altered to `false`; a fallback table whose main branch holds only a NULL partition and whose fallback branch holds a non-NULL binary partition plans the data table and `$audit_log` when the fallback branch names partitions by bytes, and the planned `$audit_log` splits read both rows back through Paimon, while both fail at planning with the legacy-name error when only the main branch names them by bytes; and a `scan.file-creation-time-millis` read of a fallback table with a non-NULL binary partition plans the main branch's file. `nativeBinaryPartitionsKeepAllPathValues`, added by #68786 for the rendering of binary partition values, now creates its table with `'partition.legacy-name' = 'false'`: under legacy names the files it planned did not exist. The new tests fail when the lookup check, the check of the files' schemas, the exemption of external paths or of NULL values is removed, when the check moves before the `COUNT(*)` splits, when the deletion vectors are not checked, when every deletion vector fails without its existence being checked, when a file whose schema uses legacy names fails without its existence being checked, when a system table read through JNI is checked under the wrapper's options, when every split is judged under the main branch, or when a split that does not come from the fallback scan is cast to a fallback split (the file-creation-time read then failed with a `ClassCastException`). The 853 tests of fe-connector-paimon run without failures (one is skipped, as before). No regression test is added: Doris cannot create a binary column, so the tables would have to come from the Spark container of the Paimon docker environment, which was not run locally.
This was referenced Oct 31, 2026
4 of 16 tasks
5 of 16 tasks
hubgeter
added a commit
to hubgeter/doris
that referenced
this pull request
Nov 5, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68786 Problem Summary: Iceberg STRING partition values equal to the literal 'null' and SQL NULL share the same legacy commit token. Even with distinct partition writers, the FE can commit the literal value as a null partition, so partition pruning misses matching rows. Preserve nullable values through dynamic, static, and mixed BE writers, transmit an optional NULL bitmap, and share decoding across data-file metadata, delete-file metadata, and identity-partition conflict filters. Retain legacy sender behavior and typed JSON nulls, and reject inconsistent commit partition metadata. ### Release note Fix Iceberg writes incorrectly treating literal STRING partition values equal to 'null' as SQL NULL. The complete correction requires upgraded FE and BE processes and applies to newly committed files. ### Check List (For Author) - Test: Unit Test / Regression test / Manual test - 150 FE writer-helper and connector-transaction unit tests passed. - BE focused unit tests are building; not yet executed. - Parquet/ORC regression output generation and comparison passed (28 query results). - ASAN BE build, FE build, formatting, header hygiene, and FE Checkstyle passed. - Both modified BE test files passed full-file clang-tidy; the final changed-line scan is in progress. - Behavior changed: Yes. New literal 'null' partitions retain non-null manifest values and conflict filters; malformed partition metadata fails before commit. - Does this need documentation: No
hubgeter
added a commit
to hubgeter/doris
that referenced
this pull request
Nov 9, 2026
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68786 Problem Summary: Iceberg STRING partition values equal to the literal 'null' and SQL NULL share the same legacy commit token. Even with distinct partition writers, the FE can commit the literal value as a null partition, so partition pruning misses matching rows. Preserve nullable values through dynamic, static, and mixed BE writers, transmit an optional NULL bitmap, and share decoding across data-file metadata, delete-file metadata, and identity-partition conflict filters. Retain legacy sender behavior and typed JSON nulls, and reject inconsistent commit partition metadata. ### Release note Fix Iceberg writes incorrectly treating literal STRING partition values equal to 'null' as SQL NULL. The complete correction requires upgraded FE and BE processes and applies to newly committed files. ### Check List (For Author) - Test: Unit Test / Regression test / Manual test - 150 FE writer-helper and connector-transaction unit tests passed. - BE focused unit tests are building; not yet executed. - Parquet/ORC regression output generation and comparison passed (28 query results). - ASAN BE build, FE build, formatting, header hygiene, and FE Checkstyle passed. - Both modified BE test files passed full-file clang-tidy; the final changed-line scan is in progress. - Behavior changed: Yes. New literal 'null' partitions retain non-null manifest values and conflict filters; malformed partition metadata fails before commit. - Does this need documentation: No
2 of 3 tasks
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
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 problem does this PR solve?
Related PR: #68532. Supersedes #68603.
Preserve external binary, UUID, and timestamp semantics across connector schemas, JDBC transport, file readers, and external writes. The tables below list the type mappings changed by this PR, including mappings that previously depended on an opt-in property.
Behavior changes
VARBINARYand preserve raw bytes, including invalid UTF-8, embedded NULs, and trailing zeros. File TVFs also apply this rule to nested binary fields.DESCRIBEand query results therefore expose binary types and hexadecimal values instead of decoded text.UUID, including nested fields, independently of binary mapping options. Native UUID inputs and canonical/compact UUID text can be written to Iceberg UUID columns; invalid text fails explicitly. UUID file reads preserve network byte order and can load directly into native Doris UUID columns.TIMESTAMPTZ; timestamps representing local wall-clock fields map toDATETIMEV2. Instant semantics also apply to MySQLTIMESTAMPand ClickHouseDateTime/DateTime64without an explicit timezone in the type name.TIMESTAMPTZpreserves the instant, not the source timezone identifier or original offset spelling.enable.mapping.varbinary/enable.mapping.timestamp_tzand file TVF propertiesenable_mapping_varbinary/enable_mapping_timestamp_tzremain accepted but no longer select the mappings below. Absent,false, andtruesettings produce the same types. Recursive mappings apply to supported ARRAY/MAP/STRUCT fields.timestamptzvalues become SQL NULL, with nullable metadata propagated even for source NOT NULL columns. MySQL retains its configured zero-date handling. Predicates mixing instant and wall-clock timestamps stay local when remote evaluation would change their meaning.--1arithmetic from comments and honors the remote connection’sNO_BACKSLASH_ESCAPESmode. SQL Server UUID range predicates stay local because its GUID ordering differs from Doris. Iceberg UUID value predicates also stay local because file bounds can otherwise prune matching UUID rows.Changed read mappings: external system → Doris
“Before” describes the previous default (mapping options absent/false). Where an opt-in mapping already existed, this PR makes that mapping unconditional.
pdenotes the effective fractional precision supported by the mapper, capped at 6 (HMS uses the catalog timestamp precision); fixed precisions are shown explicitly.ndenotes the supported declared byte bound; unqualifiedVARBINARYuses Doris's default maximum bound. Unchanged source-type mappings are omitted.BINARYSTRINGVARBINARYTIMESTAMP WITH LOCAL TIME ZONEDATETIMEV2(p)TIMESTAMPTZ(p)binarySTRINGVARBINARYfixed(n)CHAR(n)VARBINARY(n)uuidSTRINGUUIDtimestamptz(shouldAdjustToUTC=true)DATETIMEV2(6)TIMESTAMPTZ(6)BINARY(n),VARBINARY(n)/BYTESSTRINGVARBINARY(n)/VARBINARYTIMESTAMP WITH LOCAL TIME ZONE/TIMESTAMP_LTZ(p)DATETIMEV2(p)TIMESTAMPTZ(p)BINARY(n),BYTESSTRINGVARBINARY(n),VARBINARYrespectivelyTIMESTAMP_LTZ(p)DATETIMEV2(p)TIMESTAMPTZ(p)uuidonstringSTRINGUUIDtimestamp-millis,timestamp-microsDATETIMEV2(3),DATETIMEV2(6)TIMESTAMPTZ(3),TIMESTAMPTZ(6)respectivelylocal-timestamp-millis,local-timestamp-microsBIGINTDATETIMEV2(3),DATETIMEV2(6)respectivelyTIMESTAMPDATETIMEV2(6)TIMESTAMPTZ(6)UUIDUUIDVARBINARYSTRINGVARBINARYTIMESTAMP(p) WITH TIME ZONEDATETIMEV2(p)TIMESTAMPTZ(p)BINARY,VARBINARY,TINYBLOB,BLOB,MEDIUMBLOB,LONGBLOBSTRINGVARBINARYwith the supported JDBC-reported byte boundTIMESTAMP(p)DATETIMEV2(p)TIMESTAMPTZ(p)uuid, including array elementsSTRINGUUIDbyteaSTRINGVARBINARYwith the supported JDBC-reported byte boundtimestamptzDATETIMEV2(p)TIMESTAMPTZ(p)BLOBSTRINGVARBINARYwith the supported JDBC-reported byte boundTIMESTAMP(p) WITH LOCAL TIME ZONEDATETIMEV2(p)TIMESTAMPTZ(p)TIMESTAMP(p) WITH TIME ZONESTRINGin the connector; unsupported in the legacy clientTIMESTAMPTZ(p)uniqueidentifierSTRINGUUIDbinary,varbinary,imageSTRINGVARBINARYwith the supported JDBC-reported byte bounddatetimeoffset(p)STRINGTIMESTAMPTZ(p)BLOB,BINARY,VARBINARYSTRINGVARBINARYwith the supported JDBC-reported byte boundBINARY,VARBINARY,LONGVARBINARY,BLOBSTRINGVARBINARYwith the supported JDBC-reported byte boundDateTime,DateTime('zone')DATETIMEV2(0)TIMESTAMPTZ(0)DateTime64(p[, 'zone'])DATETIMEV2(p)TIMESTAMPTZ(p)uuid, including array elementsUUIDTIMESTAMP(p) WITH TIME ZONE,TIMESTAMP WITH TIME ZONEDATETIMEV2/STRING, or a metadata precision-parsing failure depending on spelling/pathTIMESTAMPTZ(p); precision defaults to 6 when absentTIMESTAMPmetadataSTRINGDATETIMEV2(6)BYTE_ARRAY, including binary fallback for legacy ENUM/BSON annotationsSTRINGVARBINARYFIXED_LEN_BYTE_ARRAY(16)STRINGduring legacy schema inference; scanner V2 already supported native UUID; opt-in binary mapping exposedVARBINARY(16)UUIDconsistentlyTIMESTAMPwithisAdjustedToUTC=trueDATETIMEV2(3/6)TIMESTAMPTZ(3/6)for millis / micros or nanosBINARYSTRINGVARBINARYBINARYwithiceberg.binary-type=UUIDSTRINGUUIDTIMESTAMP_INSTANTDATETIMEV2(6)TIMESTAMPTZ(6)OceanBase JDBC delegates to the MySQL or Oracle mapper according to its database mode and inherits the corresponding changed mappings above.
The file TVF rows apply to schema inference through supported file sources such as S3, HDFS, LOCAL, and HTTP. Existing unzoned timestamp mappings (for example MySQL
DATETIME, PostgreSQLtimestamp, Icebergtimestamp, and Paimon/FlussTIMESTAMP) remainDATETIMEV2and are omitted from the table. Precision beyond microseconds is truncated. Physical binary storage with a recognized STRING/UTF8/DECIMAL annotation retains that logical type.Changed external schema mappings: Doris → external system
UUID, including nested fieldsuuidVARBINARYbinary, including nested fieldsDATETIMEV2(p)TIMESTAMP(6); source precision discardedTIMESTAMP(p)TIMESTAMPTZ(p)TIMESTAMP_LTZ(p)TIMESTAMPTZTIMESTAMP, with UTC microsecond write transportRelease note
The external binary, UUID, and timestamp mappings listed above are determined by source semantics. The former mapping options no longer opt out of VARBINARY or TIMESTAMPTZ. Applications reading these columns may observe changed schemas, binary result formatting, and timestamp semantics. External UUIDs retain native UUID semantics rather than mapping to string or binary.
Validation
Check List (For Author)
Check List (For Reviewer who merge this PR)