morningman opened a new pull request, #65970:
URL: https://github.com/apache/doris/pull/65970
### What problem does this PR solve?
Issue Number: #65185
Related PR: #65893, #65733, #65502, #65473, #65329
Problem Summary:
Part of the catalog-SPI migration tracked in #65185, which decouples the
built-in
connectors (hive / iceberg / hudi / paimon) from FE core into loadable
plugins.
An independent clean-room review of the migrated catalog SPI raised 28
abstraction-mismatch
findings (data/predicate/schema/partition/stats/write granularity). Each was
re-verified
against the current tree in a two-stage adversarial pass (independent
investigation →
red-team). Most collapsed on inspection: 2 were refuted (#5, #17), 4 were
already fixed
upstream (#3, #4, #6, #28-core), and the bulk of the rest were dead code,
intentional
legacy parity, or estimate-only skew. This PR lands the genuinely actionable
subset, plus
a few adjacent connector runtime/build fixes discovered alongside. The
verification
workspace (the survey, seven per-category analyses, the rollup, and per-fix
design/summary
notes) is included under `plan-doc/catalog-spi-mismatch/`.
Each fix is a standalone commit; connector changes are connector-local
(fe-core keeps
shrinking, per the migration's "fe-core only sheds source-specific code"
rule).
**1. Correctness — user-visible fixes**
- **paimon time-travel read across a rename no longer crashes BE (#2).** For
a
plugin-driven read that pins a historical MVCC snapshot with a *distinct*
schema
(`FOR VERSION/TIME AS OF`, `@branch/@tag`),
`PluginDrivenScanNode.buildColumnHandles`
now resolves the connector column handles at the *pinned* schema via a new
opt-in
`ConnectorTableOps.getColumnHandles(session, handle,
ConnectorMvccSnapshot)` overload
(capability `supportsColumnHandleSnapshotPin`, default false, implemented
only by
paimon). Previously a mixed projection (a surviving column plus a renamed
one) resolved
handles against the *latest* schema, silently dropped the renamed column's
handle, and
still sent it to BE as a scan slot absent from paimon's field-id
dictionary — a BE
`StructNode` `std::out_of_range`/SIGABRT. This is a **crash fix**, not a
wrong-results
fix. iceberg was already immune (it rebuilds its field-id dictionary from
the full
pinned schema) and is untouched. A capability-gated fail-loud guard now
throws a
`UserException` (surfaced through the split-assignment error channel) if a
bound column
is present in the pinned schema but still lacks a handle, instead of
degrading into a BE
crash.
- **hudi decimal partition predicate no longer prunes matching rows (#1).**
`HudiConnectorMetadata.extractLiteralValue` now renders a `BigDecimal`
literal via
`stripTrailingZeros().toPlainString()` (gated on `instanceof BigDecimal`;
all other
literals unchanged). Before, a `decimal(8,4)` predicate rendered as
`"1.0000"` and never
string-equalled the Hive-canonical stored partition value `"1"`, so the
partition was
pruned and its rows silently dropped from the result. Mirrors the
rendering hive already
uses.
- **paimon `partition_values()` TVF handles DATE/null (#14).**
`PaimonConnectorMetadata` now puts the already-rendered spec (DATE
formatted via
`DateTimeUtils.formatDate`, genuine null normalized to the Hive
default-partition
sentinel) into the `ConnectorPartitionInfo` value map, aligning it with
hive/iceberg
which already carry decoded canonical strings. Before, the raw paimon spec
carried DATE
as an epoch-day string, which the active TVF feeder parsed and threw on —
failing the
whole `partition_values()` query — and rendered null as the internal
sentinel string
rather than SQL NULL.
**2. CBO estimate accuracy — plans only, results unchanged (#15)**
- Time-travel row-count is now estimated at the pinned snapshot instead of
the latest
table. `ConnectorStatisticsOps` gains an opt-in
`getTableStatistics(session, handle, ConnectorMvccSnapshot)` overload;
`StatementContext.getVersionedSnapshot` exposes the single versioned pin
for a table;
`PluginDrivenExternalTable.getRowCount` computes the count at that
snapshot (bypassing
and not polluting the latest-keyed cross-statement cache) and falls back
to the cached
latest estimate when a table cannot count at the snapshot or there is no
statement
context. iceberg reads the pinned snapshot's summary (`table.snapshot(id)`
vs
`currentSnapshot()`); paimon sums the pinned split row counts. This only
affects the
cardinality fed to join reorder / build-side selection; query results were
correct before
and after.
**3. User-signed-off enhancements (#10, #12, #16)**
- **iceberg column write default (#10).**
`IcebergConnectorMetadata.parseSchema` now
carries `field.writeDefault()` through the new
`IcebergSchemaUtils.writeDefaultToDorisString` (reusing the read-side
human-string form;
returns null for missing defaults, complex, and binary-like types). A v3
write default
now survives a schema refresh, so `DESCRIBE` shows it and an INSERT that
omits the column
applies it instead of writing NULL. This only populates FE `Column`
metadata and is
orthogonal to the read-default (initialDefault) BE path from #65502.
- **paimon nested STRUCT field comment (#12).** A `COMMENT` on a field
nested inside a
STRUCT column is now carried through both the paimon write path (4-arg
`DataField`) and
read path (`ConnectorType.structOf(..., comments)`), and the shared
`ConnectorColumnConverter.convertStructType` now threads per-field comment
and
nullability (connectors that leave them unset are byte-identical to
before, so only
paimon changes). The nested comment is visible via `SHOW CREATE TABLE` and
the on-disk
`$schemas.fields`; `DESCRIBE` omits nested-field comments for every table
by design, so
it is not asserted there.
- **SqlCache quiet-window gate uses a real wall clock (#16).**
`CacheAnalyzer` now computes
the quiet-window as `now - max(latestPartitionUpdateMillis)` sourced from
a new
wall-clock accessor
(`MTMVRelatedTableIf.getNewestUpdateTimeMillisForCache`), separate
from the raw version token used for BE PCache staleness. iceberg fills it
from
`last_updated_at` micros/1000 while its monotonic version marker stays in
micros
(`ConnectorMvccPartitionView` renamed to drop the misleading `Millis`
unit; pure rename,
no behavior change). Before, iceberg's ~1.7e15-micros token clamped `now`
via `Math.max`
and permanently suppressed SqlCache for a quiet iceberg table (and any
query joining one).
This is a **cache-eligibility (availability) fix**; staleness is still
guarded
independently by the unchanged version-token equality check.
**4. Connector-SPI shape convergence — refactor, no behavior change (#7, #8,
#9)**
- **#8** lifts source-specific methods off the shared transaction contracts
into narrow
opt-in capability interfaces: `RewriteCapableTransaction` (iceberg
compaction rewrite)
and `WriteBlockAllocatingConnectorTransaction` (maxcompute block
allocation) replace the
methods previously declared on `ConnectorTransaction`, and the maxcompute
write-block
concept is removed from the generic fe-core `Transaction` interface
(new `WriteBlockAllocatingTransaction extends Transaction`). Call sites
(`FrontendServiceImpl`, `ConnectorRewriteDriver`,
`PluginDrivenTransactionManager`) now
`instanceof`-gate the capability. Transaction-id granularity,
commit/rollback,
update-count, and every generic method are unchanged; the user-facing
error strings are
byte-identical.
- **#7 / #9** are javadoc-only corrections: narrow
`ConnectorWriteHandle.getWriteContext`
to "the static partition spec" (its only producer), and rewrite the
`ConnectorBucketSpec`
javadoc to drop the dead `iceberg_bucket`/`hive_hash` algorithm names and
document the
real table-level `DISTRIBUTE BY` values consumed by hive. No code change.
**5. Dead-code & comment cleanup — no behavior change (#21, #23, #25, #28)**
- Remove the never-constructed, never-read `ConnectorDeleteFile` (#21) and
`ConnectorDomain`/`ConnectorRange` pushdown abstraction (#23) from
connector-api, with
their vestigial accessors (`ConnectorScanRange.getDeleteFiles`,
`ConnectorFilterConstraint.getColumnDomains`); iceberg merge-on-read
deletes use their
own untouched path.
- Add `equals`/`hashCode`/`toString` to `ConnectorMvccSnapshot` for parity
with its
value-object siblings (#25; additive, no current Map/Set-key usage).
- Reword stale "dormant until SPI_READY_TYPES" comments in the hive
connector now that the
HMS→SPI flip is complete (#28; comment text only, verified no non-comment
lines changed).
**6. Adjacent connector runtime/build fixes**
- **paimon-scanner classpath**: bundle `hive-common` + `hive-shims-common`
(pinned to
`${hive.version}`) and `hive-standalone-metastore` into the paimon-scanner
assembly so
HMS-backed Paimon reads link — fixing BE `NoClassDefFoundError` on
`org/apache/hadoop/hive/conf/HiveConf` and
`.../metastore/api/NoSuchObjectException`
after #65733 slimmed the shared BE classpath. Build/packaging only.
- **preload-extensions classpath**: restore `net.java.dev.jna:jna` /
`jna-platform`
`5.13.0`, fixing all trino-connector cases failing with
`NoSuchMethodError com.sun.jna.Native.load` (oshi/Udev) after fe-common
went trino-free
and the older hadoop-hdfs jna won the parent-first classpath.
Build/packaging only.
- **iceberg flat MODIFY COLUMN**: a `MODIFY COLUMN` that omits `COMMENT` now
preserves the
field's existing doc instead of clearing it (threads a `commentSpecified`
flag, parity
with the nested path / legacy ops), and a complex→primitive type change is
now rejected
with a clean `Modify column type from complex to primitive is not
supported` message
instead of leaking a raw iceberg type-diff error (symmetric with the
existing
primitive→complex guard; the operation was already unsupported).
**7. Review workspace (docs, `plan-doc/`)**
- Add `plan-doc/catalog-spi-mismatch/`: the abstraction-mismatch survey,
seven per-category
analyses (A–G), the verification rollup, HANDOFF/TASKLIST, and per-fix
design/summary
notes. `plan-doc/` is already tracked on this migration branch.
Findings verified and intentionally **not** changed (refuted, already-fixed
upstream, or
intentional legacy parity — e.g. #3/#4/#5/#6/#11/#13/#17/#18/#19/#22/#24)
are recorded with
evidence in `plan-doc/catalog-spi-mismatch/analysis-00-rollup.md`. The
`ReaderType`
unification (#27) is left as an independent design task.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
- paimon time-travel mixed-projection read across a rename no longer
crashes BE (#2).
- hudi decimal partition predicate no longer prunes matching rows
(#1).
- paimon `partition_values()` TVF handles DATE/null (#14).
- iceberg column write default is surfaced for DESCRIBE and
INSERT-omitted fill (#10).
- paimon nested STRUCT field comment carried through create/read
(SHOW CREATE TABLE) (#12).
- iceberg tables become SqlCache-eligible (availability; staleness
still guarded) (#16).
- iceberg flat MODIFY COLUMN preserves an omitted comment / rejects
complex→primitive.
- Time-travel CBO row-count estimate is snapshot-accurate (plans
only; results unchanged) (#15).
- Refactor/cleanup items (#7/#8/#9/#21/#23/#25/#28) and the
classpath fixes carry no logic change.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
