[Draft] Master catalog spi 413 upgrade compat ut - #66213
Open
morningman wants to merge 92 commits into
Open
Conversation
morningman
requested review from
924060929,
CalvinKirs,
Gabriel39,
dataroaring,
englefly,
gavinchou,
liaoxin01,
luwei16,
morrySnow,
mymeiyi,
seawinde and
starocean999
as code owners
July 29, 2026 06:06
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29788 ms |
Contributor
TPC-DS: Total hot run time: 177369 ms |
Contributor
ClickBench: Total hot run time: 25.55 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This multi-month refactor needs persistent state for progress, decisions, risks, and cross-session agent handoff. Establishes a file-based tracking system including dashboard, ADR decision log, deviation log, risk register, per-stage task files, per-connector tracking, and an agent collaboration playbook covering context budget / subagent usage / handoff norms. Closes 18 design decisions (D-001..D-018) and registers 14 risks (R-001..R-014). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…T27) (apache#63582) ## Summary Lands the P0 SPI baseline for the catalog-SPI migration (master plan §3.1 / RFC §2.1), with zero impact on the already-migrated JDBC + ES connectors. - **Batch 0** (commits 1-2): SPI types + fe-core bridges — `ConnectorMetaInvalidator`, `ConnectorTransaction`, `ConnectorMvccSnapshot`, `ExternalMetaCacheInvalidator`, `ConnectorMvccSnapshotAdapter`, `PluginDrivenTransactionManager` generalization. - **Batch 1** (commit 3): DDL + Partition SPI — `ConnectorCreateTableRequest` + 4 spec POJOs, 4 new defaults on `ConnectorTableOps`, 3 new fields on `ConnectorPartitionInfo`, fe-core converter, `PluginDrivenExternalCatalog.createTable` routing. - **Batch 2** (commit 4): Import-gate + unit tests — `tools/check-connector-imports.sh` wired through exec-maven-plugin; `FakeConnectorPlugin` covering every default fall-through; routing tests for the invalidator; converter tests for all 4 partition styles + 2 bucket flavors. ## Commits - `[feat](connector) add P0 batch 0 SPI baseline: MetaInvalidator / Transaction / MvccSnapshot` (T03-T08) - `[feat](connector) wire P0 batch 0 SPI into fe-core` (T09-T12) - `[feat](connector) add P0 batch 1 SPI: CreateTableRequest + listPartitions` (T13-T20) - `[feat](connector) add P0 batch 2 gate + unit tests` (T21-T23, T26-T27) ## Test plan - [x] `mvn -pl fe-connector/fe-connector-api,fe-connector/fe-connector-spi -am compile` — SPI modules compile - [x] `mvn -pl fe-core -am compile -Dmaven.build.cache.enabled=false` — fe-core compile - [x] `mvn -pl fe-core checkstyle:check` — 0 violations - [x] `mvn -pl fe-connector validate` — import gate runs and passes (baseline clean) - [x] `mvn -pl fe-core -am test -Dtest='FakeConnectorPluginTest,ExternalMetaCacheInvalidatorTest,CreateTableInfoToConnectorRequestConverterTest,ConnectorPluginManagerTest,ConnectorSessionImplTest'` — 39/39 green - [x] `mvn -pl fe-connector/fe-connector-jdbc,fe-connector/fe-connector-es -am compile` — downstream connectors compile unchanged - [ ] JDBC regression-test suite (T24) — to be exercised by this PR's CI pipeline - [ ] ES regression-test suite (T25) — to be exercised by this PR's CI pipeline ## Tracking Full plan, decisions, and risk log live under `plan-doc/` in the repo (introduced by 6315983, already on the base branch). Per-task status: `plan-doc/tasks/P0-spi-foundation.md`. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pache#63641) P1 batch A — close out scan-node SPI consolidation while keeping migration-period fallbacks in place. Three surgical changes route `PluginDrivenExternalTable` first in the nereids translator hot paths so already-migrated SPI connectors (JDBC, ES) take the SPI route, while the existing `instanceof XExternalTable` chains remain as fallbacks for connectors still pending migration (P3–P7). - **T3** — `PhysicalPlanTranslator.visitPhysicalFileScan`: move the existing `PluginDrivenExternalTable` branch from position 8 to position 1; the 7 connector-specific branches (HMS / Iceberg / Paimon / Trino / MaxCompute / LakeSoul / RemoteDoris) stay in place as migration-period fallbacks - **T4** — `PhysicalPlanTranslator.visitPhysicalHudiScan`: add a `PluginDrivenExternalTable` branch routed to `PluginDrivenScanNode.create(...)`, threading `tableSnapshot` + `scanParams` through `FileQueryScanNode` setters; `incrementalRelation` flagged as a P3 Hudi SPI extension TODO. The new branch is unreachable today (`PhysicalHudiScan` is only built for `HMSExternalTable + DLAType.HUDI`), so this is groundwork for P3 with zero current-day runtime impact - **T5** — `LogicalFileScan`: in `computeOutput()`, add a `PluginDrivenExternalTable` branch calling new helper `computePluginDrivenOutput()` — same shape as `computeIcebergOutput`, using `getFullSchema()` + virtualColumns; in `supportPruneNestedColumn()`, add an explicit `PluginDrivenExternalTable → false` branch. Both behaviorally equivalent for JDBC/ES today since they have no hidden cols and no virtualColumns P1 batch B (T1 — delete 13 legacy `Jdbc*Client` + `JdbcFieldSchema`) is deferred to P8 because the 3 fe-core callers — `PostgresResourceValidator`, `StreamingJobUtils`, `CdcStreamTableValuedFunction` — are live CDC streaming code that requires SPI extension for `getPrimaryKeys` / `getColumnsFromJdbc` / `listTables`, which is out of P1 surgical scope. Background and tracking docs live in `plan-doc/` (Master Plan §3.2 P1, tasks/P1-scan-node-cleanup.md, decisions log). - [x] `mvn -pl fe-core -am compile -Dmaven.build.cache.enabled=false` → BUILD SUCCESS - [x] `mvn -pl fe-core checkstyle:check` → 0 violations - [x] JDBC + ES regression-test passing — baseline established in P0 / PR apache#63582 - [ ] PR CI green on this PR - [ ] Manual scan-node smoke for an SPI connector — JDBC `SELECT *` should fall into the new `PluginDrivenExternalTable` branch first 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…apache#64096) ### What problem does this PR solve? Related PR: apache#63582 (P0 — SPI baseline), apache#63641 (P1 — nereids plugin-driven routing) Problem Summary: This is **P2** of the catalog SPI migration and targets the `branch-catalog-spi` feature branch (continuing P0 apache#63582 and P1 apache#63641). It fully migrates `trino-connector` off the legacy in-tree `fe-core/datasource/trinoconnector/` implementation and onto the connector SPI module `fe-connector-trino`, making `trino-connector` the first connector to complete the SPI consumption playbook that later connectors will reuse as a template. All five batches land together so there is no intermediate state where a newly-created trino catalog cannot be serialized. **Batch A — complete the SPI surface (`fe-connector-trino` only, no fe-core changes)** - `TrinoConnectorProvider.validateProperties`: enforce the required `trino.connector.name` property at `CREATE CATALOG` time (ported from the legacy `checkProperties`). - `TrinoDorisConnector.preCreateValidation`: call `ensureInitialized()` so plugin loading + connector-factory resolution happen at catalog creation instead of being deferred to the first `SELECT`. - `TrinoConnectorDorisMetadata.applyFilter` / `applyProjection`: bridge Trino native filter/projection pushdown, reusing `TrinoPredicateConverter` to translate a Doris `ConnectorExpression` into a Trino `TupleDomain`. `remainingFilter` is conservatively returned as the original expression to match legacy behavior (conjuncts are not stripped; BE re-evaluates them). **Batch B — fe-core bridge for image compatibility** - `GsonUtils`: atomically replace the three legacy `registerSubtype` entries (`TrinoConnectorExternalCatalog` / `Database` / `Table`) with `registerCompatibleSubtype` redirects onto the `PluginDrivenExternal*` hierarchy. This must be atomic — `RuntimeTypeAdapterFactory` rejects duplicate labels, so keeping both bindings would throw at static init. Mirrors what ES/JDBC already did. - `PluginDrivenExternalCatalog.gsonPostProcess`: extract a `legacyLogTypeToCatalogType()` helper that maps `Type.TRINO_CONNECTOR` → `"trino-connector"`; the generic `name().toLowerCase()` would otherwise produce the wrong `"trino_connector"` (underscore) that `CatalogFactory` does not recognize. - `PluginDrivenExternalTable.getEngine()` / `getEngineTableTypeName()`: add `trino-connector` branches that preserve the legacy engine-name / table-type display across `SHOW TABLE STATUS` and `information_schema`. **Batch C — flip the switch** - Add `"trino-connector"` to `CatalogFactory.SPI_READY_TYPES` so catalog creation routes through the SPI path. **Batch D — remove legacy code** - Drop the `instanceof TrinoConnectorExternalTable` scan branch in `PhysicalPlanTranslator` (the `PluginDrivenExternalTable` SPI branch already handles it). - Drop `case "trino-connector"` in `CatalogFactory`. - Delete `fe-core/datasource/trinoconnector/` (10 files) and the now-dead legacy `TrinoConnectorPredicateTest`. - Route the `TRINO_CONNECTOR` db-build case in `ExternalCatalog` to `PluginDrivenExternalDatabase` (mirrors the migrated JDBC case). - **Retained for image compatibility**: the `InitCatalogLog.Type.TRINO_CONNECTOR` and `TableType.TRINO_CONNECTOR_EXTERNAL_TABLE` enums, the GsonUtils redirects, and the `MetastoreProperties` trino-connector entry. **Batch E — tests + tracking docs** - 29 JUnit 5 unit tests over the plugin-free converters: - `TrinoPredicateConverterTest` — `ConnectorExpression` pushdown trees → Trino `TupleDomain` (EQ / range / NE / IN / IS [NOT] NULL / AND / OR, Slice encoding), plus graceful degradation to `TupleDomain.all()` on null/unsupported input. - `TrinoTypeMappingTest` — Trino SPI type → Doris `ConnectorType` (scalars, decimal precision/scale, timestamp precision clamp, array/map/struct, unsupported-type failure). - `TrinoConnectorProviderTest` — `validateProperties` fast-fails when `trino.connector.name` is missing/empty. - No Trino plugin/cluster required; plugin-dependent paths remain covered by the existing `external_table_p0/p2` `trino_connector` regression suites. - Sync the migration tracking docs under `plan-doc/` (already carried on this feature branch since P0). **Net effect**: 28 files, +1025 / −2681 (~1656 LOC net removed). Old FE images holding legacy trino catalogs / databases / tables deserialize onto the `PluginDrivenExternal*` hierarchy through the GsonUtils string-name redirect, with engine-name display preserved. **Deferred (follow-ups, not in this PR)**: - `trino_connector_migration_compat` regression test (old-image deserialization) — requires a running cluster + Trino plugin + docker, unavailable in this dev environment; tracked as a CI/cluster follow-up. - The plugin-install documentation update lives in the `doris-website` repo and is handled separately. ### Release note None ### Check List (For Author) - Test - [x] Unit Test — 29 new tests in `fe-connector-trino` (predicate converter / type mapping / property validation). - [ ] Regression test — existing `trino_connector` suites cover plugin paths; the new old-image compat regression is deferred to a CI/cluster follow-up. - [ ] 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 - Behavior changed: - [x] No. Internal routing moves from the legacy fe-core path to the SPI path; image compatibility, engine-name display, and pushdown semantics all mirror the legacy behavior. All batches land together, so there is no serialization-gap window. - Does this need documentation? - [x] Yes. The trino-connector plugin-install doc update is a follow-up in the `doris-website` repo. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
…tch design (hybrid, T02-T08) (apache#64143) ## Proposed changes testing with apache#64146 P3 of the catalog-SPI migration (base: `branch-catalog-spi`). Migrates the **hudi** connector following the **hybrid** strategy (D-019): harden the dormant HMS-over-SPI hudi connector to correctness parity, build a test baseline, and write the per-table dispatch design — **all behind the closed gate** (`SPI_READY_TYPES` unchanged). >⚠️ **No user-visible behavior change.** The SPI hudi path stays dormant (gate closed); hudi queries continue to use the legacy `HMSExternalTable.dlaType=HUDI` path. This PR removes correctness blockers ahead of the live cutover (deferred to P7 / batch E). ### What's included **Correctness fixes (hardening dormant code, behind gate):** - **T02** — fix hudi JNI `column_types` double bug: emit full Hive type strings (was Doris bare type names, losing precision/scale/subtypes) and send `column_names`/`column_types`/`delta_logs` as typed lists end-to-end (was comma join/split, which shattered `decimal(10,2)` / `struct<...>`). Matches the BE `hudi_jni_reader.cpp` contract (names `,` / types `#` / delta `,`). - **T04** — fail loud on time-travel / incremental read in the SPI `visitPhysicalHudiScan` branch (was silently returning the latest snapshot / silently full-scanning). - **T05** — real EQ/IN partition pruning in `HudiConnectorMetadata.applyFilter` (was a placeholder that ignored predicates and unconditionally switched the partition source from Hudi-metadata to HMS); faithfully mirrors `HiveConnectorMetadata.applyFilter`. - **T07** — column-name casing fix in `avroSchemaToColumns` (top-level lowercase, mirroring legacy `HMSExternalTable`). **Test baseline (all three connector modules started P3 with 0 tests):** - `fe-connector-hudi` (33): type-mapping / schema-parity (COW/MOR golden) / table-type / partition-pruning / scan-range. - `fe-connector-hms` (12): shared Hive-type-string parser tests. - `fe-connector-hive` (14): file-format / partition-pruning (mirrors T05). - COW/MOR schema is **type-agnostic** (golden parity vs legacy `initHudiSchema`); table type only affects scan planning. **Decisions / design (code-grounded, design-only):** - **T03** — defer `schema_id`/`history_schema_info` field-id evolution to batch E (DV-006; not a model-agnostic SPI fix). - **T06** — keep MVCC/snapshot SPI defaults (opt-out) + document (DV-007). - **T08** — `tableFormatType` dispatch design memo + **D-020**: single `hms` catalog per-table routing via a new backward-compatible `ConnectorMetadata.getScanPlanProvider(handle)` (per-table provider seam); refines D-005. The keystone gap is split into M1 (identity consumption, fe-core reads `tableFormatType` as an opaque string) and M2 (scan routing). ### Deferred to batch E / P7 (not in this PR) Gate flip (`SPI_READY_TYPES += hms/hudi`), fe-core `tableFormatType` consumption (M1+M2 implementation), live cutover, delete legacy `datasource/hudi/`, full incremental/time-travel/MVCC, Iceberg-on-hms via SPI (needs P6 `IcebergScanPlanProvider`), cluster/runtime validation. ### Verification Per task tracking, each code batch landed with: per-module compile + checkstyle 0 (incl. test sources) + connector import-gate pass + new unit tests green. The two most recent commits are docs-only (`plan-doc/`); the code is unchanged since the last green batch. Gate stays closed → the dormant SPI path is unreachable at runtime → zero live-path risk. CI re-verifies. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…core + make fe-core odps-free (T07-T09) (apache#64300) Follow-up to apache#64253 (the MaxCompute catalog-SPI cutover). After the cutover a `max_compute` catalog deserializes to `PluginDrivenExternalCatalog` and no legacy `MaxComputeExternal*` object is ever instantiated, so the legacy MaxCompute subsystem in fe-core is dead code. This removes it and makes fe-core's dependency tree fully odps-free. **1. Remove legacy subsystem** (`7a4db351100`) - Delete 20 fe-core files: `datasource/maxcompute/*` (incl. `MCTransaction`, `MaxComputeScanNode`/`Split`), the MaxCompute sink/insert/txn plumbing, and 2 legacy-only tests. - Clean ~21 reverse-reference sites (imports + dead `instanceof`/visitor/rule branches), keeping every `PluginDriven`/connector sibling branch and the image/replay keep-set (GsonUtils compat strings; `TableType`/`TransactionType`/`TableFormatType`/`InitCatalogLog.Type` `MAX_COMPUTE` enums; block-id thrift). - Rewire 3 tests; e.g. `FrontendServiceImplTest`'s block-id RPC test now mocks the generic `Transaction` SPI, since `getMaxComputeBlockIdRange` reads the PluginDriven connector transaction. **2. Make fe-core odps-free** (`409300a75b8`) - Drop the two odps deps from `fe-core/pom.xml`. - Move `MCUtils` from fe-common into `be-java-extensions/max-compute-connector` (its only consumer after the removal); keep `MCProperties` (odps-free constants) in fe-common. - Drop `odps-sdk-core` from fe-common — it was also leaking netty/protobuf transitively to fe-common's own `DorisHttpException`/`GsonUtilsBase`, so declare `netty-all` + `protobuf-java` directly (proper dependency hygiene). **3. Doc-sync** (`f8c305765e8`) — plan-doc PROGRESS/HANDOFF/deviations/design tracking notes. - `mvn -pl :fe-core -am test-compile` (main+test) passes; checkstyle 0 violations; connector import-gate passes. - `grep -rn com.aliyun.odps fe/fe-core/src` → empty. - `mvn -pl :fe-core dependency:tree | grep odps` → empty (no odps, direct or transitive). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…he#64446) (apache#64446) Migrates the legacy in-tree Paimon catalog (fe-core `datasource/paimon`, `metacache/paimon`, `property/metastore/*Paimon*` and their reverse references) onto the catalog SPI as a self-contained `fe-connector-paimon` plugin, following the maxcompute full-adopter + cutover template. `paimon` is added to `SPI_READY_TYPES`, so a Paimon catalog now deserializes to `PluginDrivenExternalCatalog` and all five functional areas — normal-table read, system-table read, DDL, MTMV/MVCC/time-travel (procedures are a doc-only no-op: fe-core has none) — flow through the generic `PluginDriven*` bridge with no Paimon-specific branch in fe-core. Paimon is the first lakehouse full-adopter to exercise the MVCC / sys-table / MTMV SPI surfaces, which become the reusable template for the upcoming iceberg/hudi migrations, so the cutover is held to byte-parity with the legacy path. The legacy `datasource/paimon/*` classes are left in place (now dead on the cutover path) and removed in a follow-up, mirroring the maxcompute cutover -> P4 removal. DefaultConnectorContext) Source-agnostic seams so any connector can express these capabilities; every new method is a default no-op/identity, so hive/iceberg/jdbc/maxcompute/ trino/es are byte-for-byte unaffected. - MVCC/time-travel: `resolveTimeTravel(session, handle, ConnectorTimeTravelSpec)` + `applySnapshot(...)` (threads a pinned snapshot into the handle before planScan); new immutable `ConnectorTimeTravelSpec` (SNAPSHOT_ID / TIMESTAMP / TAG / BRANCH / INCREMENTAL) that fe-core extracts from SQL; `ConnectorMvccSnapshot` carries `schemaId` for schema-evolution-aware time travel. - System tables: `listSupportedSysTables` / `getSysTableHandle`; fe-core composes the `{base}${sys}` reference name. - Scan: proportional split-weight (`getSelfSplitWeight`/`getTargetSplitSize`), COUNT(*) pushdown (`getPushDownRowCount`, 7-arg `planScan`), native-read accounting (`isNativeReadRange`), `getDeleteFiles` (VERBOSE merge-on-read), `ignorePartitionPruneShortCircuit` (predicate-driven connectors keep genuine-null partitions). - Metadata: `ConnectorPartitionInfo.fileCount` + `SUPPORTS_PARTITION_STATS` (rich SHOW PARTITIONS); `ConnectorColumn.withTimeZone` (WITH_TIMEZONE DESC marker, independent of the timestamp_tz mapping flag); connector cache control (`invalidateTable`/`invalidateAll`, `schemaCacheTtlSecondOverride`). - `ConnectorContext`: six engine-side hooks the connector must not implement itself (`loadHiveConfResources`, `vendStorageCredentials`, `getBackendStorageProperties`, `normalizeStorageUri` x2, `getStorageProperties`), bound in `DefaultConnectorContext` over fe-core's single-source-of-truth credential/URI/HiveConf utilities (no re-ported logic that could drift). The metastore-side counterpart of the fe-filesystem StorageProperties SPI: a Paimon catalog resolves its backend (hive/dlf/rest/jdbc/filesystem) by ServiceLoader dispatch on `paimon.catalog.type` instead of a fe-core switch. - `fe-connector-metastore-api`: hadoop/SDK-free `MetaStoreProperties` contracts (HMS/DLF/REST/JDBC/FileSystem) carrying only Map/scalar facts. - `fe-connector-metastore-spi`: `MetaStoreProvider<P>` + first-hit `MetaStoreProviders` dispatcher + five impls, registered via META-INF/services. `HmsMetaStorePropertiesImpl` reproduces the legacy `buildHmsHiveConf` key set and parity-critical ordering (storage overlay before the kerberos block; username/sasl last). Adding a backend = one provider class + one services line (no central enum/switch). - ServiceLoader is loaded with the SPI interface's own (plugin) classloader, NOT the thread-context CL: at CREATE CATALOG the static init first fires on an FE worker thread whose TCCL is the FE app loader, so a 1-arg load would cache an empty provider list process-wide. - fe-core bridges add `initExecutionAuthenticator(List<StorageProperties>)` so HDFS-Kerberos doAs is wired on the plugin path (legacy `initializeCatalog` is dead). Full read + DDL + partitions + statistics + system tables + MVCC/time-travel across all five metastore flavors, planning native (ORC/Parquet) vs JNI reads. - `PaimonCatalogFactory` (pure flavor switch: Options + Hadoop Configuration + HiveConf, classloader-pinned for child-first loading), `PaimonCatalogOps` (injection seam over the remote Catalog for offline-testable metadata), `PaimonConnectorMetadata` (~9 -> 28 methods: schema latest/at-snapshot, sys tables, MVCC/time-travel, DDL, partitions, statistics, table descriptor). - `PaimonScanPlanProvider` (~6 -> 30 methods): predicate/projection pushdown, native-vs-JNI routing, large-file sub-splitting with deletion vectors, COUNT(*) collapse, schema-evolution field-id dictionary, `$ro` unwrap to base FileStoreTable. - New: `PaimonSchemaBuilder` (CREATE TABLE), `PaimonIncrementalScanParams` (@incr), `PaimonTableResolver` (sys/branch-aware handle -> Table), `PaimonLatestSnapshotCache` + `PaimonSchemaAtMemo` (per-catalog caches on the long-lived connector). - `PaimonTypeMapping` gains the write direction (Doris -> Paimon) for CREATE TABLE. - NEW `PluginDrivenMvccExternalTable` (generic MvccTable + MTMV host: the runtime class for paimon tables), `PluginDrivenMvccSnapshot`, `PluginDrivenSysExternalTable` + `systable/PluginDrivenSysTable`. Behavior is selected by `ConnectorCapability` (SUPPORTS_MVCC_SNAPSHOT, SUPPORTS_PARTITION_STATS) — no paimon branch in fe-core. - `PluginDrivenScanNode`: MVCC pin threaded at every handle-consumption site, sys-table time-travel guard, COUNT pushdown, connector-agnostic EXPLAIN re-emission. - Cutover: `CatalogFactory` adds `paimon` to `SPI_READY_TYPES` and drops the built-in case; `GsonUtils` removes the 7 built-in `Paimon*` subtype registrations and adds replay compat so persisted `PaimonExternalCatalog` (all 5 flavors) / `...Database` -> `PluginDriven*` and `PaimonExternalTable` -> `PluginDrivenMvccExternalTable` on deserialization (existing clusters upgrade without losing catalogs). - Nereids/DDL: `PhysicalPlanTranslator` drops the `PaimonScanNode` branch; `UserAuthentication`, `CreateTableInfo`, `ShowPartitionsCommand` generalized; `Env.getDdlStmt` renders LOCATION/PROPERTIES for SHOW CREATE (paimon engine, no credential leak). - fe-filesystem: NEW `HdfsFileSystemProperties` / `HdfsConfigFileLoader` (typed HDFS; defaults-laden BE map vs defaults-free FE Hadoop-config map so it cannot clobber a co-bound object store's fs.s3a.* during a multi-backend merge); S3 MinIO support + legacy tuning defaults; OSS/OBS/COS ANONYMOUS provider-type; new `FileSystemFactory.bindAllStorageProperties` / `FileSystemPluginManager.bindAll`. - NEW `fe-kerberos` leaf module: dependency-free `AuthType` / `KerberosAuthSpec` facts shared across modules without pulling in Hadoop. - Packaging: NEW `fe-connector-paimon-hive-shade` relocates `org.apache.thrift` for HMS 2.3.7; the paimon plugin-zip excludes fe-thrift/libthrift (provided parent-first) to avoid a split `TBase` across loaders; self-contained bundle (hadoop-aws, AWS-SDK, hadoop-hdfs-client). `paimon.version` (1.3.1) pinned as the single property across FE connector / BE paimon-scanner / preload-extensions for FE<->BE Table/Split serde. - BE `PaimonJniScanner.getPredicates()` null-predicate backstop (no-filter scan; pairs with the FE producer now always emitting an empty predicate list). Parity fixes surfaced by the SPI cutover (each restores legacy semantics): - Storage/credentials: canonical `s3.*`/`oss.*` keys -> `fs.s3a.*`/`fs.oss.*` (STORAGE-CREDS); canonical `AWS_*` BE creds (STATIC-CREDS-BE); per-table vended DLF/STS token (REST-VENDED); `oss/cos/obs/s3a` -> `s3://` for data + deletion-vector paths (URI-NORMALIZE); external `hive.conf.resources` hive-site.xml (HMS-CONFRES); real doAs on filesystem/jdbc + wrapped HMS read RPCs (KERBEROS-DOAS). - Reads/types: paimon columns forced nullable (READ-NOTNULL); `VARCHAR(65533)` not widened to STRING (VARCHAR-BOUNDARY); dotted `enable.mapping.varbinary` / `.timestamp_tz` keys (MAPPING-FLAG-KEYS); per-type partition-value rendering (NATIVE-PARTVAL); genuine `\N` not coerced to NULL (PARTITION-NULL-SENTINEL); field-id history dict for rename-safe native reads (SCHEMA-EVOLUTION); paimon-native split encode under `enable_paimon_cpp_reader` (CPP-READER). - Planning/perf: restore the `force_jni_scanner` escape hatch (FORCE-JNI-SCANNER); intra-file native sub-splitting (NATIVE-SUBSPLIT); precomputed COUNT(*) row count (COUNT-PUSHDOWN); table row count for CBO / SHOW TABLE STATUS (TABLE-STATS). - Time travel / DDL: `FOR TIME AS OF` under CST/PST session zones (TZ-ALIAS); resolved + validated `driver_url` for jdbc catalogs (JDBC-DRIVER-URL); reject a local-only name conflict on CREATE TABLE (CREATE-TABLE-LOCAL-CONFLICT). plan-doc P5 design / task-list / HANDOFF / deviation tracking notes. - ~70 new FE unit suites: paimon connector (offline recording fakes: `RecordingPaimonCatalogOps` / `FakePaimonTable`), fe-core PluginDriven* MVCC / sys-table / scan-node / split-weight, metastore-spi dispatch + per-backend properties, fe-filesystem HDFS/MinIO/anonymous, fe-kerberos. - checkstyle 0 violations; connector import-gate passes; FE main+test compile clean. - End-to-end paimon behavior is docker-gated (`enablePaimonTest=true`); the five functional areas are covered by the `external_table_p0/paimon` regression suites, green on the latest CI run for this branch.
… make fe-core paimon-SDK-free (T29) (apache#64653) Follow-up to apache#64446 (the Paimon catalog-SPI cutover). After the cutover a `paimon` catalog deserializes to `PluginDrivenExternalCatalog` and its tables to `PluginDrivenMvccExternalTable`, so no legacy `Paimon*` catalog / table / scan-node object is ever instantiated — the entire legacy Paimon subsystem in fe-core is dead code. This PR removes it and makes fe-core's dependency tree fully paimon-SDK-free (zero `org.apache.paimon` imports, the 5 paimon maven deps dropped). Mirrors the P4 maxcompute cutover → legacy-removal (apache#64256). **1. Remove legacy subsystem + sever reverse-refs** (`7632a074e4b`) - Delete the dead fe-core paimon files: `datasource/paimon/*` (all 5 flavor catalogs, `PaimonExternalTable`, `PaimonMetadataOps`, `PaimonUtil`, `source/PaimonScanNode` / `PaimonSplit` / `PaimonPredicateConverter` / `PaimonSource` / `PaimonValueConverter`, `profile/*`), `datasource/metacache/paimon/*`, `systable/PaimonSysTable`, and the legacy-only tests. - Sever the live reverse-references to the deleted classes, keeping every `PluginDriven` / connector sibling branch and the image/replay keep-set: drop the `PAIMON` db-creation switch arm in `ExternalCatalog`; the paimon engine routing in `ExternalMetaCacheMgr` / `ExternalMetaCacheRouteResolver`; the dead `PAIMON_EXTERNAL_TABLE` branch in `Env.getDdlStmt` (keep the LIVE `PLUGIN_EXTERNAL_TABLE` SHOW CREATE LOCATION/PROPERTIES rendering); the `PaimonSysExternalTable` branch in `UserAuthentication`; the legacy clauses + `handleShowPaimonTablePartitions()` in `ShowPartitionsCommand` (keep `hasPartitionStatsCapability()` and the `TableType.PAIMON_EXTERNAL_TABLE` enum). Landed as one compiling unit (mirrors the P4 apache#64300 precedent: `PaimonUtils` still called the removed `ExternalMetaCacheMgr.paimon()`). - Inline the `getPaimonCatalogType()` string literals (`hms`/`filesystem`/`dlf`/`rest`/ `jdbc`) to decouple the still-consumed thin metastore-property classes from the deleted `PaimonExternalCatalog`. **2. Make fe-core paimon-SDK-free** (`32de7d16702`) - Strip the dead paimon-SDK catalog-building cluster from `AbstractPaimonProperties` + the 5 flavors (`initializeCatalog` / `buildCatalogOptions` / `appendCatalogOptions` / `getMetastoreType` / `normalizeS3Config` / … — 0 live callers; the plugin path builds catalogs connector-side). Keep every LIVE SDK-free duty: `warehouse` `@ConnectorProperty`, the Kerberos `executionAuthenticator` doAs wiring (read by `PluginDrivenExternalCatalog`), property normalization/validation, `getPaimonCatalogType`, `Type.PAIMON`. - Vended credentials: delete `PaimonVendedCredentialsProvider` + its test + the `VendedCredentialsFactory` `case PAIMON`. Its SDK methods were dead — reachable only via the iceberg-only `getStoragePropertiesMapWithVendedCredentials`; the real paimon vended path is the connector's `PaimonScanPlanProvider.extractVendedToken` (moved off fe-core at cutover). Relocate its one LIVE duty (the REST "skip static storage map" gate) to a new SDK-free `MetastoreProperties.isVendedCredentialsEnabled()` (base=false, `PaimonRestMetaStoreProperties`=true); `CatalogProperty` routes iceberg through its provider (byte-identical) and everything else through the metastore-props method. - pom: drop `paimon-core` / `-common` / `-format` / `-s3` / `-jindo` from `fe-core/pom.xml`. `fe/pom.xml` `paimon.version` is kept (fe-connector-paimon + BE paimon-scanner still consume it). **3. Doc-sync** (`58160cbcb56`, `895e214b2bd`) — plan-doc HANDOFF / PROGRESS / connectors / P5-T29 legacy-removal design tracking notes. - `mvn -pl :fe-core -am test-compile` (main + test) passes; checkstyle 0 violations; connector import-gate (`tools/check-connector-imports.sh`) passes. - `grep -rn org.apache.paimon fe/fe-core/src` → empty (main and test). - `mvn -pl :fe-core dependency:tree -Dincludes=org.apache.paimon` → empty (no paimon dep, direct or transitive). - End-to-end paimon regression (`external_table_p0/paimon`, docker `enablePaimonTest=true`) passed on this branch.
…kerberos (apache#64655) **P3b: consolidate the drifted Kerberos/Hadoop authentication implementations into the new top-level neutral leaf module `fe-kerberos`** as the single source of truth. Done as 3 commits: 1. **trino → JDK** (`4a740e1`) — replace the only external dependency in the auth path, trino's `KerberosTicketUtils`, with a JDK-only (`javax.security.auth.kerberos`) byte-for-byte equivalent, so the kerberos path is trino-free. 2. **relocate** (`8898e15`) — move the 13 `fe-common` `security.authentication.*` classes to `org.apache.doris.kerberos.*` in `fe-kerberos`; retarget all consumer imports (fe-core + 3 be-java-extensions scanners); merge the duplicate `AuthType`. 3. **unify interface** (`5e3e896`) — merge the two competing `HadoopAuthenticator` interfaces (fe-common's `PrivilegedExceptionAction` variant vs fe-filesystem-spi's `IOCallable` variant) into the single fe-kerberos one, and delete fe-filesystem-hdfs's own `KerberosHadoopAuthenticator`/`SimpleHadoopAuthenticator` copies (which had drifted from the canonical impls). `DFSFileSystem` now routes through the shared authenticators. `fe-kerberos` remains a top-level neutral leaf (no dependency cycle). HDFS filesystem access now uses the same authenticators as the HMS path (restoring parity). Two intentional behavior changes in fe-filesystem-hdfs: simple / no-`hadoop.username` now runs as remote user `hadoop` (was: FE process user, direct); kerberos uses the shared `LoginContext` + 80%-lifetime refresh. fe-filesystem-hdfs 79/0/0 (+fe-kerberos/spi), checkstyle 0, connector import-gate clean, whole-repo grep for the removed symbols = 0. >⚠️ docker kerberos e2e (HDFS kerberized + HMS) NOT yet run — the real gate; UGI login can't be exercised in unit tests.
…pache#66004 storage facade) Also records an upstream-owned regression found while validating: apache#66004 itself breaks ForwardToMasterTest by double-nesting the /rest/v2/manager/node/* JSON payload (its parent 0dde273 is green, and so is this branch before the rebase). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ill matches Issue Number: None Related PR: None Problem Summary: e1a8963 ("Support Iceberg Docker stacks on Apple Silicon") rewrote the CONTAINER_UID assignment in custom_settings.env as CONTAINER_UID="${CONTAINER_UID:-doris--}" so the value could be overridden from the environment. Three pipelines patch that file with an exact-literal sed: sed -i "s/^CONTAINER_UID=\"doris--\"/CONTAINER_UID=\"doris-external--\"/" ... (the External Regression pipeline script, regression-test/pipeline/vault_p0/run.sh and regression-test/pipeline/cloud_p0/run.sh). The rewritten line no longer matches, so the sed silently became a no-op, the file still expanded to "doris--", and run-thirdparties-docker.sh refused to start with "Must set CONTAINER_UID to a unique name in custom_settings.env". External Regression and vault_p0 aborted before running a single case; cloud_p0 swallows the error but never starts kafka, so its routine load cases fail for no visible reason. Keep the assignment literal so the sed keeps matching, and apply the environment override on the lines around it. The macOS workflow documented in docker-compose/iceberg/README.md (CONTAINER_UID=dorismac- bash run-thirdparties-docker.sh ...) keeps working unchanged. - Test: Manual test - CI path: ran both real sed variants (External Regression's and vault_p0/cloud_p0's) against the file, then sourced it with no CONTAINER_UID in the environment -> "doris-external--", guard not hit - macOS path: sourced without sed and with CONTAINER_UID=dorismac- -> "dorismac-", override still honored - Upstream default: sourced without sed and without the env var -> "doris--", still rejected by the guard as before - Repeated sourcing is idempotent; sourcing under "set -euo pipefail" is safe and leaves no CONTAINER_UID_FROM_ENV behind; bash -n passes - Behavior changed: No, this restores the pre-e1a8963dad CI behavior while keeping the environment override added by that commit - Does this need documentation: No Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… var The "Generate incremental licenserc" step died before running any of its own code: An error occurred trying to start process '/usr/bin/bash' ... Argument list too long The changed-file list was passed to the step through the CHANGED_FILES environment variable. Linux caps a single argv/envp string at MAX_ARG_STRLEN (32 pages = 128KiB); once a PR touches enough files the variable exceeds that and execve() of the step's shell fails with E2BIG. On PR apache#64304 the list is 1713 files / 147771 bytes. The existing guard only counts files (>= 3000 falls back to the full config), so a PR can slip under the count and still blow the byte limit -- apache#64304 changes 2184 files in total. Any PR past roughly 1300 changed files hits this. Write the list to a file under RUNNER_TEMP and pass the path instead. That drops the step output from ~147800 bytes to 185 and removes the size ceiling entirely; the count guard stays as the "too big, just do a full check" policy. RUNNER_TEMP rather than the workspace, so the full-config fallback path does not license-check the scratch file. The list still never reaches a shell as interpolated text, so the injection hardening from apache#63486 is preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Once the license step can start at all, the check itself reports 12
files. Three groups:
Accidentally committed build/run artifacts, removed: fe/1.tree (334KB of
mvn dependency:tree output) and fe/1.pom, both from the P5 paimon
commit; paimon-scanner/${sys:logPath}, a file created by a log4j config
that never expanded its property, holding a junit run's log; and
.claude/wf-d2-cache-recon.js.
Working documents that stay on the branch, given the standard header:
reset_submodule.sh and the six plan-doc files that .licenserc.yaml's
"**/*.md" rule does not already cover (five .workflow.js and one
.java.txt).
connector-metadata-methods.txt is added to paths-ignore instead.
ConnectorMetadataSurfaceTest.readBaseline() treats every non-blank line
as a recorded method signature -- it skips blank lines but not comments
-- so a header would be read as 16 phantom signatures and fail the test.
This matches how stageUtilTest.txt and net_snmp_normal are already
handled.
Verified: of the 1711 add/modified files, 0 now lack a header. The full
tree scan flags only 126 .avro and 17 extensionless data files, all
pre-existing on master, which the full check on master already passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pendency review The slim in-repo fe-connector-hms-hive-shade declares org.codehaus.jackson:jackson-mapper-asl 1.9.2 explicitly, so the dependency review now sees GHSA-c27h-mcmw-48hv (critical) and GHSA-r6j9-8759-g62w (high) and fails every PR against this branch. Nothing new ships. Those codehaus classes already ride inside the prebuilt org.apache.doris:hive-catalog-shade jar that this module replaces, and Hive's metastore JSONMessageDeserializer -- which the HMS notification-event path builds on -- runs on them. Building the shade in-repo only makes the dependency visible to the scan. Legacy Jackson 1.x is EOL and neither advisory has a fixed version, so there is no upgrade to take. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o fe-connector-paimon Upstream apache#65984 adds a relation-scoped `@options('scan.snapshot-id'='1', ...)` clause for paimon. Its fe-core half is catalog-neutral and merged as-is; its paimon half landed in the fe-core paimon subsystem this branch deleted (P5-T29), so it moves into the connector. `@options` is just a sixth time-travel selector, so it rides the existing SPI pipeline rather than a new one: ConnectorTimeTravelSpec gains Kind.OPTIONS carrying the raw map, paimon's resolveTimeTravel validates the keys and freezes a mutable selector (scan.mode=latest, a tag, a wall clock) into an immutable pin, applySnapshot threads it onto the handle, and resolveScanTable applies it via PaimonScanParams.applyOptions. fe-core never inspects an option key. BindRelation rejects the clause up front through a new SUPPORTS_SCAN_PARAM_OPTIONS capability: @options only reaches a connector through the MVCC pin path, so a table that never enters it would silently answer a historical query with latest data. Two upstream changes needed no port: initialSelectedPartitions -> NOT_PRUNED and computeOutput's getFullSchema(scanParams) arm are already covered, because LogicalFileScan resolves partitions and schema against THIS reference's own version and an explicit pin yields empty (scan-all) partition maps. Upstream's per-relation resolve-once cache is likewise provided by StatementContext's version key, which includes the option map. apache#65984 also widens paimon system tables from "no scan params at all" to a per-view capability matrix. The generic pipeline for that already exists (resolveSysTableSnapshotPin + applyMvccSnapshotPin); it only needed a per-system-table question, so ConnectorScanPlanProvider gains supportsSystemTableIncrementalRead/Options and the PluginDrivenScanNode guard now names the missing capability instead of refusing wholesale. paimon's applySnapshot stops returning system handles unchanged so the pin reaches the view. Three independent hardenings ride along: the @incr null-reset widens from two keys to paimon's whole inherited read-state family; a $binlog @incr read vetoes COUNT(*) pushdown (its reader packs an UPDATE_BEFORE/AFTER pair into one logical row); and the JNI-forced system-table set picks up row_tracking, now sourced from the same capability table. Deliberately NOT ported, with the reasons recorded in plan-doc/HANDOFF.md so a later rebase does not re-add them: upstream re-adds jni.enable_file_reader_async to the backend option list, but no BE/JNI consumer for it exists anywhere, including upstream master itself (apache#65955 deleted it) -- this branch already guards that with a test; and the selector-free @options preload exemption, which would need the connector's option vocabulary before binding and only affects warmup latency. Tests: fe-connector-paimon 421/421, fe-connector-api 110/110, fe-connector-iceberg 1151/1151, fe-core 8335 tests / 0 failures (2 pre-existing errors: upstream apache#66004's NodeInfo JSON regression and HFUtilsTest needing network), full FE install + checkstyle green. Upstream's PaimonScanParamsTest is ported to the connector; its resolution cases now run against real local paimon tables because no connector module carries mockito. Three mutations verified RED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-connector-iceberg Upstream apache#65870 is behaviour-inert: it extracts IcebergScanNode's schema-info initialization into a @VisibleForTesting helper, documents that an identity partition column added by a newer spec can still be stored physically in files written by an older spec, and pins the all-column reader schema with one FE unit test. This branch deleted the fe-core iceberg subsystem, so the fe-core edit is moot; what matters is whether the connector upholds the same invariant. It does, by a different route, so no capability had to be migrated: identity partition columns are unioned over ALL partition specs into path_partition_keys, FileQueryScanNode.classifyColumn turns those into TColumnCategory.PARTITION_KEY (non-file slots), and BE resolves through the field-id dictionary only the slots it decodes from the file. The upstream test therefore cannot be transplanted verbatim — it asserts an all-column dictionary the connector deliberately prunes. Instead pin the property the connector actually relies on, which had no multi-spec coverage: getIdentityPartitionColumns must union every spec, not just the current one. Mutating table.specs() to table.spec() turns the new test red and leaves the other 62 green. Also record the reasoning at the dictionary construction site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he snapshot file
`@options('scan.tag-name'='t')` failed at ANALYZE time on a table whose snapshot had
expired but whose tag survived:
Snapshot file s3://.../tbl_time_travel_expired_tag/snapshot/snapshot-1 does not exist.
It might have been expired by other jobs operating on this table.
SnapshotManager.snapshot(:128)
PaimonCatalogOps$CatalogBackedPaimonCatalogOps.snapshotSchemaId(:342)
PaimonConnectorMetadata.resolveTimeTravel(:656) <- case OPTIONS
A paimon tag owns a RETAINED copy of its snapshot, which outlives the ordinary
snapshot/snapshot-<id> file. The TAG case has always taken both the snapshot id and the
schema id off that copy, which is why `FOR VERSION AS OF 't'` reads an expired-but-tagged
version fine. The OPTIONS case used the tag only to look up the ID and then re-derived the
schema id through snapshotManager().snapshot(id) -- i.e. through the file that is gone.
Take both ids off the retained copy, exactly as the TAG case does. A tag-valued
`scan.version` lands here too, because resolveOptions canonicalizes it into scan.tag-name.
`pinnedSnapshotId`'s tag arm is now unreachable, so it is removed rather than left to
re-hide the bug; its javadoc records why a tag never reaches it.
Not covered by this fix, and untested anywhere today: `@options('scan.snapshot-id'=<expired>)`
dies earlier, inside PaimonScanParams.resolveOptions' tryTravelOrLatest, and surfaces paimon's
raw text instead of the connector's "can't find snapshot by id: N".
Found by External Regression 1007447 (paimon_time_travel.qt_expired_tag_options_count),
which passed on every other PR the same day and failed only here.
Tests: new PaimonConnectorMetadataOptionsTagPinTest drives a real local paimon table with a
real tag and asserts WHICH seam the schema id came from, not just its value -- against the
pristine pre-fix code 2 of its 3 cases go red (schemaId 99 from the snapshot-file seam
instead of 7 from the tag); the third (an explicit snapshot-id pin, which must still resolve
through the snapshot) stays green either way. fe-connector-paimon 424/424.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t at latest
`t$audit_log@options('scan.tag-name'='top_cp0')` failed during ANALYSIS with
"Unknown column 'old_name' in 'table list' in PROJECT clause", while every base-table
@options form on the same table and the same tag bound fine.
A system table is not an MvccTable, and BindRelation returns from handleMetaTable BEFORE
StatementContext.loadSnapshots, so the statement's MVCC map never holds an entry for the
$-suffixed relation. MvccUtil.getSnapshotFromContext therefore answers empty by
construction and LogicalFileScan.computePluginDrivenOutput silently bound the LATEST
schema -- while the scan itself read the pinned snapshot, because the scan half
(resolveSysTableSnapshotPin + applySnapshot's sys arm) does resolve the pin off the source.
The visible symptom is the loud one: a column renamed after the pin cannot bind. The
dangerous one is silent -- a column whose TYPE changed binds at the latest type over
pinned data (in the failing suite `victim` is BIGINT at latest and STRING at the tag).
The port of upstream apache#65984 carried the scan half and the per-system-table capability
matrix, but not upstream's analysis half (PaimonSysExternalTable.getFullSchema(scanParams)
plus LogicalFileScan's arm for it). Its claim that "computeOutput's getFullSchema(scanParams)
arm is already covered" holds for the base table -- which does reach loadSnapshots, which is
exactly why the base-table assertions pass -- and not for the system table.
- PluginDrivenSysExternalTable resolves the pin off its SOURCE and MEMOIZES it per
selector. The memo is load-bearing, not an optimization: a MUTABLE selector
(scan.mode=latest, a wall-clock scan.timestamp-millis) is re-evaluated against the LIVE
table on every loadSnapshot, so resolving once per consumer would let a commit landing
between binding and scanning hand them different versions -- re-creating the very skew
this closes. It is keyed the way StatementContext.versionKeyOf keys, field by field:
TableScanParams defines neither value equality nor toString(), so an object- or
toString-keyed memo would miss every lookup and resolve twice.
- buildColumnHandles gets the same sys fallback as pinMvccSnapshot. Fixing only the
binding would leave the PROJECTION at latest, so the pinned old-name slot would miss
the latest-keyed handle map, be silently dropped, and crash BE on the field-id dict --
the shape of ExtReg 1004351.
- The paimon connector answers both 3-arg metadata calls for a sys handle from the
option-applied VIEW (the metadata-side twin of resolveScanTable). Routing them through
schemaAt would return the BASE table's historical fields and drop the view's own
columns, e.g. $audit_log's leading `rowkind`.
Scope is wider than the failing query: OPTIONS_SYSTEM_TABLES also covers $ro and $binlog,
which derive their row type from the base table the same way and have no coverage today.
Found by External Regression 1007447
(test_paimon_schema_time_travel_matrix.order_qt_audit_log_options_tag_schema), which passed
on every other PR the same day and failed only here.
Tests: new PluginDrivenSysExternalTableScanPinTest (6) covers source delegation, resolve-once
per selector, independent selectors, no-selector, a declining connector, and a non-MVCC
source; reverting the memo key to object identity turns the resolve-once case red.
PluginDrivenScanNodeSysTablePinTest's two positive cases now assert the delegation and that
loadSnapshot is NOT called again there. fe-core datasource.plugin/scan/mvcc 379/379,
nereids logical+analysis 256/256, fe-connector-paimon 424/424.
Not yet verified end to end: the regression suite itself needs a docker run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e nested-DDL capability test_iceberg_on_hms_gateway_ddl_parity has never passed, in any environment: it failed on its first executable assertion in all four builds that ran it (1006034, 1006199, 1006647, 1007447), so lines 89-145 have never executed anywhere. The suite asserted that nested column DDL SUCCEEDS on an iceberg-on-HMS table, on the stated premise that the dedicated catalog "succeeds on the very same table". That premise was carried over from REST-catalog experience and was never checked against a metastore -- every other nested-DDL suite in the repo is REST-backed (iceberg_schema_change and the partition-evolution suites all use iceberg.catalog.type='rest'). Nothing exercised nested struct evolution against HMS. Against HMS it does not hold, and cannot be made to hold from Doris. Iceberg's HiveTableOperations rewrites the HMS column list from the iceberg schema on every commit, so adding a nested field changes an EXISTING hive column's type string, which HMS refuses unless it is primitive widening (hive.metastore.disallow.incompatible.col.type.changes, default true since Hive 2.0). From UpdateSchema.commit() downward both catalogs run identical iceberg code against the same metastore, so the refusal cannot differ between them. The FE stack from 1007447 confirms the forwarding fix itself works -- HiveConnectorMetadata.addNestedColumn -> IcebergConnectorMetadata.addNestedColumn -> IcebergNestedColumnEvolution.addColumn -> SchemaUpdate.commit -> HiveTableOperations.doCommit -> InvalidOperationException from HMS. The branch moved the failure from "Doris refuses" to "the metastore refuses". So assert the relation the suite is named for rather than an absolute capability: run the same eight-statement sequence through BOTH catalogs, on two identical tables (one each, so the first run cannot change the schema the second sees), classify each outcome into an environment-independent verdict, and require the two to agree. This keeps catching the actual regression -- a gateway that refuses the statement itself instead of forwarding it gets REFUSED_BY_DORIS while the dedicated catalog does not, so the lists diverge, and a second assertion rejects REFUSED_BY_DORIS outright. An unrecognized message is filed as OTHER and fails rather than being silently accepted as parity. The cross-catalog visibility assertions are kept and run whenever the metastore does permit the change, so an environment that allows nested struct evolution strengthens the suite rather than changing it. The comment-parity half (SHOW CREATE TABLE / SHOW TABLE STATUS / information_schema) is untouched -- it is a pure read path the metastore has no opinion about. Both product fixes also keep their unit coverage: HiveConnectorMetadataSiblingDelegationTest (16 cases, incl. everyPerHandleMethodForwardsAForeignHandleToTheSibling) and HiveConnectorMetadataTableCommentTest (4 cases). Verified: groovy parse OK; the classifier dry-run maps the real 1007447 HMS message to REFUSED_BY_METASTORE, the pre-fix gateway message and master's "Nested add column operation is not supported for this table type." both to REFUSED_BY_DORIS, and an unknown message to OTHER. Not yet run end to end -- lines 89-145 still need a docker run to execute for the first time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, not latest
`t$audit_log@options('scan.tag-name'='top_cp0')` bound fine after de87be4 but then failed
at PLAN time with "Column old_name not found in table top_timeline$audit_log"
(FileQueryScanNode.setColumnPositionMapping:315). That commit closed two of the three consumers
of a sys table's pin and missed the third, so the failure moved one stage later rather than away:
External Regression 1007447 failed the same suite line during ANALYSIS ("Unknown column
'old_name' in 'table list' in PROJECT clause"), 1007887 fails it at the scan node.
A system table is not an MvccTable and BindRelation returns from handleMetaTable BEFORE
StatementContext.loadSnapshots, so MvccUtil.getSnapshotFromContext answers empty BY CONSTRUCTION
for a $-suffixed relation. Three places on the scan path need the pin, and each needs its own
fallback off the source table:
- pinMvccSnapshot -> resolveSysTableSnapshotPin (already had it)
- buildColumnHandles (de87be4 added it)
- resolvePinnedFullSchema (this commit)
Behind getPinnedFullSchema/getPinnedBaseSchema, resolvePinnedFullSchema still went through the
context lookup, got empty, and fell back to super -- i.e. the LATEST schema -- while the slots
were bound and the column handles built at the PINNED one. setColumnPositionMapping then indexed
the pinned slots into the latest column list.
The loud half is the reported one: `old_name`, renamed to `MixedName` after the tag, has no index
and the query dies. The silent half is worse and is why this is a correctness fix rather than a
message fix -- a column that merely MOVED or changed type maps to the wrong file column and BE
reads wrong data with nothing raised. In the failing suite `victim` is BIGINT at latest and
STRING at the tag.
Fix: route a PluginDrivenSysExternalTable through its own getFullSchemaAt(tableSnapshot,
scanParams) -- the exact method LogicalFileScan.computePluginDrivenOutput binds from -- so
binding, column handles and position mapping share the sys table's memoized pin and cannot
disagree. The instance is the same one for all three: generateTupleDesc carries the
LogicalFileScan's table onto the tuple descriptor, and the scan node's selectors are forwarded
from the same fileScan, so the memo key matches field for field. A reference with no pin (or a
connector that declines the selector) resolves empty and getFullSchemaAt returns getFullSchema(),
which is byte-for-byte what super.getPinnedFullSchema()/getPinnedBaseSchema() returned before --
getBaseSchema(false) is visibleColumns(getFullSchema()) and neither is overridden in the plugin
hierarchy. Both call sites already null-check, so the no-schema path is unchanged.
Tests: PluginDrivenScanNodePinnedSchemaTest gains sysTablePositionMapsAgainstItsOwnPinNotLatest
(the pinned schema reaches getPinnedFullSchema, and the MVCC-context route -- which answers empty
by construction -- is never consulted) and sysTablePinnedBaseSchemaDropsHiddenColumns
(getBaseSchema(false) parity on the pinned schema, since it feeds numOfColumnsFromFile). Removing
the new arm turns both red; restoring it turns both green. fe-core datasource.scan/plugin pin
suites 18/18.
Not yet verified end to end: test_paimon_schema_time_travel_matrix needs a docker run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ually emits
test_iceberg_on_hms_gateway_ddl_parity failed in External Regression 1007887 on the plain-hive
guard at line 195: it expected the refusal to contain "not supported", but got
"Nested column path is only supported for Iceberg tables: s.b".
The product behavior is correct. AlterTableCommand admits nested column paths only for a target
that reports SUPPORTS_NESTED_COLUMN_SCHEMA_CHANGE -- i.e. the gateway routes by TABLE capability,
not by catalog type -- and raises that message otherwise (AlterTableCommand:179, pinned by
AlterTableCommandTest:187). The expectation was master's wording ("Nested add column operation is
not supported for this table type."), a string that appears nowhere in this branch's tree, so the
assertion was vacuous-by-accident here and would have passed on any message containing those two
words.
It surfaced only now because the line had never run. The suite died at its first executable
assertion in all four earlier builds (1006034, 1006199, 1006647, 1007447); 31ae926 reworked
the parity probe into an environment-independent verdict, the probe passed for the first time in
1007887 -- "gateway: [REFUSED_BY_METASTORE], dedicated: [REFUSED_BY_METASTORE]", parity held --
and execution reached line 195 for the first time in the suite's history.
Also teach classifyOutcome the same message. It recognized only "not supported" as
REFUSED_BY_DORIS, and that is precisely the wording a gateway which stopped recognizing its
iceberg tables would NOT produce today -- it would produce "only supported for Iceberg tables"
and be filed as OTHER. Still red, but naming the metastore's absence rather than the regression
the suite exists to catch. Both wordings now classify as REFUSED_BY_DORIS, so the verdict stays
accurate whichever side of the rename the code is on.
Verified: brace/paren/triple-quote balance checked; the two edits are the only behavioral change,
the comment-parity half is untouched. Not yet run end to end -- lines 195+ still need a docker run
to execute for the first time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
run-thirdparties-docker.sh assumes a Linux host in three places, and each
one aborts the run on macOS:
- IP_HOST detection only knows `ip` and `hostname -I`, and macOS has
neither, so IP_HOST came out empty. Darwin now gets 127.0.0.1, which is
also the only correct answer there: the "host" the `network_mode: host`
services join is the Docker Desktop VM rather than the Mac, and Docker
Desktop forwards the Mac's loopback into it. A LAN address would leave
the VM and arrive where nothing listens.
- ensure_hosts_alias rewrote /etc/hosts unconditionally, so every run
needed sudo -- which on macOS has no passwordless default and no tty to
prompt on. It now builds the file it wants, compares, and escalates only
when the alias is missing or points somewhere else.
- mysql:5.7.36 publishes a manifest list holding linux/amd64 alone, so an
Apple Silicon host resolves no manifest and the pull fails. That takes
hive down with it, because the default JuiceFS metadata URI makes mysql
an implicit hive dependency. Pinned to linux/amd64 so the host emulates
the one image that exists.
Separately, the iceberg entrypoint linked /opt/spark/{bin,sbin}/* into
/usr/local/bin with a bare `ln -s`. A container's writable layer survives
a stop, so on `docker start` those links are still present, `ln` fails,
and `set -e` takes the entrypoint down with it -- spark-iceberg could only
ever be recreated, never restarted. Now `ln -sf`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing it An iceberg table created with `COMMENT '...'` persisted no comment at all. The COMMENT clause and the PROPERTIES map are two DISTINCT fields on the create request -- CreateTableInfoToConnectorRequestConverter fills request.getComment() from the clause and request.getProperties() from PROPERTIES(...) -- and iceberg's createTable forwarded only the latter, the same gap legacy IcebergMetadataOps.performCreateTable had. Every reader downstream (getTableComment, the COMMENT clause of SHOW CREATE TABLE, information_schema.tables.TABLE_COMMENT, SHOW TABLE STATUS) therefore reported a table with no comment, through a dedicated iceberg catalog and through an HMS gateway catalog alike. Fold the clause into the `comment` table property in createTable. Resolution mirrors the fix already shipped for paimon (PaimonSchemaBuilder.build): an explicit properties["comment"] WINS, preserving the legacy persisted-comment behavior including a deliberately empty one (keyed on containsKey, not on emptiness), else the COMMENT clause is used. A blank clause writes nothing: nereids defaults CreateTableInfo.comment to "" when the clause is omitted, so stamping it unconditionally would add a noise `"comment" = ""` to the PROPERTIES of every iceberg table Doris creates. Also fixes two defects this unmasked in test_iceberg_on_hms_gateway_ddl_parity, whose comment half seeds the comment with a plain COMMENT clause and so had been asserting against a table that never carried one: - information_schema is PER-CATALOG: `information_schema.tables` resolves inside whichever catalog is current and lists only that catalog's tables, so the TABLE_CATALOG predicate can only narrow rows already scoped to the current catalog, never reach across to another one. The suite read both catalogs from a single session position, so the non-current one silently returned zero rows and failed as an NPE. Each read is now taken from inside its own catalog. - Added size() assertions on those reads, so a future regression to zero rows reports what actually went wrong instead of an NPE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y cluster and the dead storage/AWS-credential surface (apache#66210) ### What problem does this PR solve? Issue Number: apache#65185 Problem Summary: Part of the catalog-SPI migration (apache#65185). Review round 21, targeting `branch-catalog-spi`. This round is one self-contained work line: finish the architectural goal "**`fe-core` holds no property parsing**" for `org.apache.doris.datasource.property`. Everything here is deletion of code that is unreachable on this branch — nothing moves and nothing is migrated, because every successor is already live connector-side. `fe-core` loses ~860 lines and gains none. The investigation, the per-package verdicts, the rejected alternatives and the rolling handoff are included under `plan-doc/fecore-property-cleanup/`. The two packages under `datasource/property/` were investigated together but got **different** verdicts, and are deliberately not treated as one job: | package | verdict | reason | | --- | --- | --- | | `metastore/` (4 files, 333 lines) + the orphaned `ConnectionProperties` | deleted outright | unreachable at runtime; its successor has been live for some time — nothing needs to move | | `common/` (2 files, 237 lines) | **stays in `fe-core`**, only its provably dead half is cut | it serves *internal* storage (cold-storage `StoragePolicy`, cloud `StorageVault`), not external datasources; every relocation target is dependency- or classloader-illegal | **1. The metastore property cluster is deleted, not migrated** `datasource/property/metastore/` (4 files) plus the `ConnectionProperties` base it orphans. The successor is already running: `fe-connector-metastore-api` `MetaStoreProperties`, `fe-connector-metastore-spi` `MetaStoreProviders.bind` and `Connector.deriveStorageProperties`. The cluster was unreachable. Its registry held only `TRINO_CONNECTOR`, whose factory returns a bare `MetastoreProperties` without ever calling `initNormalizeAndCheckProps` — so it parsed nothing, and `createInternal`, `getDerivedStorageProperties`, `getExecutionAuthenticator` and the `StorageAuthenticatorBridge` all had zero callers. `CatalogProperty` was the sole importer and offered two doors: `checkMetaStoreAndStorageProperties` (no callers repo-wide) and `resolveDerivedStorageDefaults`, whose metastore branch runs only when `pluginDerivedStorageDefaultsSupplier` is null — and `PluginDrivenExternalCatalog` installs that supplier unconditionally. Nothing here was Gson-persisted, so no image or replay compatibility is at stake (note `MetastoreProperties.Type` is unrelated to the persisted `InitCatalogLog.Type` / `InitDatabaseLog.Type` constants of the same name). `resolveDerivedStorageDefaults` now **throws** when the supplier is unwired instead of deriving nothing. That preserves today's behavior, where the metastore parse threw on this path. Deriving an empty map would silently drop the iceberg `warehouse` → `fs.defaultFS` bridge and, because the setter deliberately does not reset caches, would cache the under-derived `StorageBindings` for good. `CatalogPropertyPluginStorageDerivationTest` gains a case pinning that, and its stale mutation comments are retargeted at mutations that still exist. **2. The callerless AWS provider-instance arm is dropped** `StorageAdapter.getAwsCredentialsProvider()` and its two helpers build a live AWS SDK v2 credentials object for FE-side SDK clients; `AwsCredentialsProviderFactory.createV2`, `createDefaultV2` and the single-arg `getV2ClassName` existed only to serve that arm. Both consumers — the catalog connectivity testers and `IcebergAwsClientCredentialsProperties` — were removed earlier in this migration along with the whole `datasource/connectivity` package, so nothing calls them here anymore. **This one is a deliberate trade, not merely dead code.** It is still live on `apache/doris` master, where both consumers exist; it reads as dead only on this branch. Removing it means future upstream edits to that region will surface as modify/delete conflicts on rebase, to be resolved by keeping the deletion. The investigation first flipped its own recommendation to *do not delete* for exactly that reason (see the `[doc](catalog) record that the dead AWS provider arm is live upstream` commit); the owner then accepted the trade, and only after that was the deletion carried out. Deliberately **kept**, because they are not part of that arm: `getAwsCredentialsProviderMode()` and the `s3CredentialsMode` field, and `AwsCredentialsProviderFactory.getV2ClassName(mode, boolean)` with its two env probes. Those emit the provider class-name string that reaches BE via `AWS_CREDENTIALS_PROVIDER_TYPE` and the hadoop `fs.s3a.aws.credentials.provider` map, and `AzureGuessRoutingParityTest` pins the mode accessor. Folding them into the look-alike `fe-filesystem-s3-base` twin was considered and **rejected**: the two implementations differ in two live behaviors (the hadoop string gains a `ProfileCredentialsProvider`; the accepted mode strings widen), and no test in the repo pins the emitted string — so the swap would have shipped a regression green. Three connector comments naming the deleted methods are reworded; the iceberg twin is now the only implementation of the provider-instance mapping. **3. The dead storage doors on the catalog base are removed** Five callerless members of `ExternalCatalog` — `getHadoopProperties`, `getConfiguration`, `buildConf`, `buildHadoopConfiguration`, `ifNotSetFallbackToSimpleAuth` — together with the `cachedConf` / `confLock` fields they cached through, and three of `CatalogProperty` — `getHadoopProperties`, `getBackendStorageProperties`, `getOrderedStorageAdapters` — with their two lazily populated fields. `getConfiguration` was already marked deprecated "until connector SPI extraction is complete", which it now is. The point is not the 135 lines. `initStorageAdapters` is now reachable only through `getStorageAdaptersMap` and `getEffectiveRawStorageProperties`, both called from `PluginDrivenExternalCatalog`, so "**`fe-core` storage has a single entrance**" holds by construction rather than by audit — which is also what backs the fail-loud branch in item 1. **4. Two test-infrastructure notes (no code)** `plan-doc/fe-core-ut-runtime-problem.md` records, with measurements rather than guesswork, why a full `fe-core` run takes 3h+: `reuseForks=false` (deliberate, to avoid singleton conflicts) means a fresh JVM per class at ~5.5s startup over 1225 classes, `forkCount` comes from `fe_ut_parallel` which defaults to 1 unless `FE_UT_PARALLEL` is set, and nereids accounts for 72% of test execution time. It also records what the FE UT pipeline actually runs — TeamCity `Doris_Doris_FeUt` invokes the repo's own `run-fe-ut.sh --coverage`, passes `-Dmaven.test.failure.ignore=true`, and forces the exit code back to 0 when a run has more than 6000 tests with at most 10 failures and 10 errors, so a green FeUt build is not the same as zero failures. Both notes are directions only, filed for a separate branch — raising `FE_UT_PARALLEL` is untested and risks fork contention over ports, BDB directories and temp paths. ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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? --> Local verification, stated at the coverage it actually has: - repo-wide grep for every deleted symbol: clean. - full reactor **clean** `test-compile` **including test sources** (no `-Dmaven.test.skip`), after each of the four deletions: BUILD SUCCESS. Deletion changes cannot be trusted to incremental compilation, so `fe-core/target/{classes,test-classes}` was removed before each run. - `fe-core` `checkstyle:check`: 0 violations. - targeted suites: 95 `fe-core` tests including the Gson replay suites; 110 `fe-connector-api` tests with `connector-metadata-methods.txt` unchanged; 52 storage-adapter and catalog-property tests (1 pre-existing `@Disabled` in `LocationPathTest`). - **the full `fe-core` suite was not run to completion.** It was still going at 3h29m after 1232 test classes and was stopped by the owner rather than finished, so the claim is "no failures among the 1232 classes executed", **not** "the suite passes". Its one failure, `ForwardToMasterTest.testAddBeDropBe` (`ClassCastException: JSONObject → JSONArray`), reproduces identically on a clean HEAD with these changes stashed, so it is pre-existing. - **no e2e was run** (no cluster available here); this PR adds no regression case. All four deletions remove unreachable code, and the storage-binding path they sit next to (iceberg hadoop `warehouse` → `fs.defaultFS`) is covered by unit tests only. CI is relied on for the rest. - no BE file is touched by this PR, so BE was not rebuilt. - Behavior changed: - [x] No. Every deleted member is callerless on this branch. The one semantic change — `resolveDerivedStorageDefaults` throwing when `pluginDerivedStorageDefaultsSupplier` is unwired — is not reachable in production wiring, since `PluginDrivenExternalCatalog` installs that supplier unconditionally, and it replaces a path that already threw. The one accepted forward cost is stated in item 2: modify/delete rebase conflicts against upstream master in the `StorageAdapter` region. - Does this need documentation? - [x] No. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… a manifest-carried API version gate for all four plugin families (apache#66211) ### What problem does this PR solve? Issue Number: apache#65185 Problem Summary: Part of the catalog-SPI migration (apache#65185). Review round 22, targeting `branch-catalog-spi`. One self-contained work line: give the four plugin families — CONNECTOR, FILESYSTEM, AUTHENTICATION, LINEAGE — an API version check that can actually reject a plugin. The design, the version-change runbook and the eight-task implementation plan are included under `plan-doc/designs/`. **1. The check that existed was true by construction** `ConnectorProvider.apiVersion()` could never reject anything. The SPI interface is listed parent-first, every plugin zip excludes the SPI jar, and no shipped connector overrides the method — so the `default` body executed on a plugin's behalf is the kernel's own code, and the number it returns is the kernel talking to itself. Raising `CURRENT_API_VERSION` and the default together admits every stale plugin; raising only the constant rejects every plugin, including freshly built ones. Neither is a check. The other three families had none at all. `apiVersion()`, `CURRENT_API_VERSION` and the three comparisons against it are deleted. **2. One number, physically distributed to both sides of the comparison** Each family declares its API version once, as a maven property in its parent pom (`fe/fe-connector`, `fe/fe-filesystem`, `fe/fe-authentication`, and `fe/fe-core` for LINEAGE). That single property flows to: - a maven-filtered resource inside the family's SPI module — what the kernel expects; - the `<manifestEntries>` of every jar built from that pom — what a plugin declares. Same build, same property, so the two can never disagree by accident; across builds they disagree exactly when they should. There is no second number, so no test has to keep them in sync. All four families start at `1.0`. Families are derived, not enumerated: the token `connector` implies the kernel resource `/META-INF/doris/connector-plugin-api-version.properties` and the manifest attribute `Doris-Connector-Plugin-Api-Version`. A fifth family follows the convention on both sides and needs no new code in the gate. **3. The gate** `ApiVersionGate` (new, in `fe-extension-loader`, family-neutral) compares **MAJOR only**. Minor and patch are ignored in both directions, which is sound only because "major" is defined as **any** change to the SPI surface, additions included — a compatible minor can never change the set of API elements a plugin can reach. A jar that declares nothing is refused: fail-closed is what makes the check meaningful for plugins built before this existed. `DirectoryPluginRuntimeManager#loadAll` takes the gate as a **mandatory** fifth parameter, not an optional one — an unwired gate would be the same silently-true check this PR removes. It runs after `loadClass` but before `asSubclass`/`newInstance`, so an incompatible plugin's constructor never executes and the operator gets a message naming both versions instead of a `ClassCastException`. Directory loading is the only production path and classpath `ServiceLoader` builtins never pass through it, so they are exempt structurally rather than by a flag. All four families are wired. LINEAGE loads at start-up, and one refused plugin still cannot stop FE. AUTHENTICATION loads lazily and reports "no factory for this type" from a different place than the load, so the rejection reason is carried into that exception at both call sites — otherwise a refused plugin is indistinguishable from one that was never installed. **4. Four SPI surface baselines** One per family, freezing what that family's plugins implement or call — including `fe-extension-spi`'s `Plugin` / `PluginFactory` / `PluginContext` in all four, so a change there turns all four red at once. Signatures are recorded **with their return type**: a changed return type is a major change by the same definition, and the older name-and-parameters-only baseline (`connector-metadata-methods.txt`) cannot see it. No test can prove someone actually bumped the property — a test sees the current state, never the delta — so the failure message says, in the same commit as the diff, that this is a major change. The four baselines cannot carry a license header (every non-blank line is read back as a signature), so they join `connector-metadata-methods.txt` in `.licenserc.yaml`. **5. Two build facts, both found by trying rather than by reasoning** - **maven-build-cache hashes a module from its sources, dependencies and `<build><plugins>`, NOT from `<properties>`** — and the filtered resource's source text is the literal placeholder. AUTHENTICATION and LINEAGE were going to ship with the property alone, since they have no in-tree plugin zips, which would have put their version outside every hashed input. Bumping 1.0 → 2.0 then produced an identical module checksum (`4c1d34a6e04836c6` both times) and a cached jar still declaring `api.version=1.0`. All four families now carry the `<manifestEntries>` block, and the checksum tracks the bump (`a388a287b5a7e172` → `cfb1d83e91b92b98`). Worth recording how this was caught: adversarial review out-voted it 2:1 as "not a defect" on the reasoning that a major bump always comes with an SPI surface change — the one reviewer who actually ran the A/B build was the one who was right. - `fe-connector-es` and `fe-connector-trino` never excluded `fe-filesystem-api` from their plugin zips, shipping a duplicate that parent-first loading guarantees is never read. Now excluded, so all eight connector zips agree. Plan docs are updated to match: D-009 ("the API version never bumps within this plan") is marked superseded, and the master plan's §2.3 rule is replaced — the bump criterion is now "any SPI surface change, additions included", not "signature change or method removal". ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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? --> Local verification, stated at the coverage it actually has: - full reactor `package` with the **build cache disabled** and test sources compiled (`-Dmaven.build.cache.enabled=false`, no `-Dmaven.test.skip`): BUILD SUCCESS. `test-compile` is not sufficient here — `fe-connector-hms` compiles against a shade artifact that binds to `package` — and the cache must be off for the reason in item 5. - `checkstyle:check` on all six touched modules: 0 violations. - tests: `fe-extension-loader` 19 (the gate's decision rule, and the loader's behaviour on real plugin jars assembled by the test with and without the attribute); `fe-core` 56 targeted (each family's wiring); authentication handler 20. - the produced artifacts were read back rather than assumed: `doris-fe-connector-es.jar` carries `Doris-Connector-Plugin-Api-Version: 1.0` alongside the inherited `Implementation-*` entries, and `doris-fe-connector-spi.jar` carries `api.version=1.0` rather than an unfiltered placeholder. - **no e2e was run** (no cluster available here) and this PR adds no regression case. The behaviour that would want one — an operator's pre-existing plugin zip being refused — is covered by the unit tests that build real jars, not by a deployment test. - no BE file is touched by this PR, so BE was not rebuilt. - Behavior changed: - [ ] No. - [x] Yes. <!-- Explain the behavior change --> A directory-loaded plugin jar that declares no API version, or one whose major differs from this FE's, is now refused at load time; previously nothing was ever refused. Fail-closed is deliberate — accepting an undeclared version would leave the check meaningless for exactly the plugins built before it existed — and rebuilding the plugin against this tree is the fix. In-tree plugins need no change: they inherit the property from their family's parent pom, so the number appears in exactly one place. Classpath `ServiceLoader` builtins are unaffected, since they do not go through directory loading. - Does this need documentation? - [ ] No. - [x] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> Not on the website yet, and not in this PR. Third-party plugin authors now have to stamp one manifest attribute, so this becomes user-facing documentation at the point where external plugins are documented at all — which this branch has not reached. Until then the runbook lives with the design: `plan-doc/designs/2026-07-29-plugin-api-version-check-design.md` covers how to tell major from minor, the Doris-side steps for each, what a third-party author must write, and how to diagnose a plugin that failed to load. ### 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 --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rcement onto the connector SPI Upstream apache#66112 (8305fe7) makes an iceberg `MERGE INTO` reject a target row that more than one source row matches, independently of `enable_strict_consistency_dml`. It did that in fe-core classes this branch no longer has (`LogicalIcebergMergeSink`, `PhysicalIcebergMergeSink`, `planner.IcebergMergeSink`, `IcebergMergeCommand`, `IcebergUpdateCommand`), so the rebase brought in the BE half and the thrift field (`TIcebergMergeSink.require_merge_cardinality_check`) with no FE producer: BE resolved the unset field to false and its duplicate-match validation was dead code. `UPDATE` and `MERGE INTO` synthesize the same sink here, so the statement kind is carried the way upstream carries it - one boolean stamped at the plan builder: ExternalRowLevel{Merge,Update}PlanBuilder (true / false) -> Logical/PhysicalExternalRowLevelMergeSink -> RequestPropertyDeriver (MERGE keeps the merge distribution even with enable_strict_consistency_dml off - BE can only see the duplicates when both matches reach one instance) -> PhysicalPlanTranslator -> PluginDrivenTableSink -> ConnectorWriteHandle#isRequireMergeCardinalityCheck (new default-false SPI method) -> IcebergWritePlanProvider stamps TIcebergMergeSink. The flag is kept off the existing `WriteOperation` axis on purpose: it is a statement-level SQL requirement, not a sink dialect, and the engine also reads it for the distribution decision. Every other connector keeps the default false, so no non-iceberg write changes. Also updates PhysicalExternalRowLevelMergeSinkTest for the behaviour apache#66112 changed in a hunk that merged cleanly: with `enable_iceberg_merge_partitioning` off the sink now asks for a DistributionSpecMerge that routes only the DELETE images by row id, instead of hashing every row by a row id that is NULL on the insert side (which serialized all inserts onto one exchange channel). The test still encoded the old DistributionSpecHash and was failing. Tests: unit coverage for each hop - origin (both polarities), deriver, translator threading, and the thrift stamp; 4-way mutation check confirms each hop's test goes red when its link is broken. FE 173 tests green in the touched classes; checkstyle 0 violations. E2E: regression-test/.../iceberg/write/test_iceberg_write_merge_duplicate_source_negative (un-gated by apache#66112) is the end-to-end gate and needs a docker run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he refresh onto the plugin bridge Upstream apache#66112 (8305fe7) fixed three CREATE TABLE defects in fe-core's legacy paimon subsystem, which this branch has migrated into the generic plugin bridge. The upstream code merged cleanly but is inert here, so each is re-implemented where it now belongs. All three are connector-agnostic on this branch, not paimon-only. 1. Reject an unsupported CTAS destination BEFORE publishing metadata. Upstream moved the sink construction ahead of `Env.createTable`; that hunk is on this branch, but `UnboundTableSinkCreator` admits ANY PluginDrivenExternalCatalog, so a CTAS into a connector with no write path (paimon / es / hudi / trino) still created the remote table and only failed later in PhysicalPlanTranslator. The fallback then dropped the table BY NAME - which cannot tell this statement's table from a concurrent creator's table of the same name, i.e. it can drop someone else's. The CTAS overload of createUnboundTableSink now asks the connector-level write provider first. A gateway connector answers it (hive/iceberg/jdbc/maxcompute return a provider), so only the genuinely write-less connectors are refused, with the same wording the translator used. 2. Refresh the FE table-name cache on an IF NOT EXISTS no-op. `PluginDrivenExternalCatalog#createTable` returned true BEFORE the post-create cache invalidation, so a table created out-of-band (another engine / another FE) stayed invisible to SHOW TABLES and to the following SELECT. A neighbouring probe happened to self-heal it, which made the guarantee accidental; it is now explicit. 3. Report a concurrent creator instead of swallowing it. The existence probe and the remote create are not atomic. The paimon connector forwarded `isIfNotExists()` as paimon's `ignoreIfExists`, so a creator that won the race in between was silently no-op'd and this statement looked like the creator - a CTAS would then INSERT into, and on failure roll back, the winner's table. Paimon now always creates with `ignoreIfExists=false` (as upstream's fe-core arm now does) and the bridge turns the resulting failure back into an IF NOT EXISTS no-op returning true, after re-probing that the table really is there. Without IF NOT EXISTS the failure still surfaces. Tests: bridge tests for both no-op arms and both race polarities, a new UnboundTableSinkCreatorTest for the CTAS admission gate, and the paimon DDL tests updated to the new contract (they encoded the pre-apache#66112 passthrough). Mutation check: removing the cache refresh or the admission gate turns the respective tests red. 76 tests green in the touched classes; checkstyle 0 violations. E2E: regression-test/.../paimon/test_paimon_ctas_atomicity_negative (un-gated by apache#66112) covers all three and needs a docker run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pache#66112) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctor SPI Upstream 794d514 (apache#65991) added `INSERT [OVERWRITE] ... PARTITION(col='val')` for hive. All five files it touched are legacy fe-core hive code that this branch deleted, so the rebase dropped everything except the InsertUtils hunk. The e2e suite it added, test_hive_write_static_partition.groovy, survived byte-identical and cannot pass without this port. Worse, the generic sink made the gap SILENT. UnboundTableSinkCreator already passes staticPartitionKeyValues for every PluginDrivenExternalCatalog, so hive reaches bindConnectorTableSink with a static spec that upstream's legacy path never received. BindSink then excludes the partition column from the bound columns (so the arity check passes) and NULL-fills it -- and the hive BE derives the partition DIRECTORY from that row value, mapping NULL to the literal __HIVE_DEFAULT_PARTITION__ (be/src/exec/sink/writer/vhive_table_writer.cpp:441). Upstream errored out here; this branch wrote to the wrong partition, silently. Four changes, one per owner: 1. HiveWritePlanProvider now declares requiresMaterializeStaticPartitionValues(). This is THE data fix: it makes BindSink re-project the PARTITION literal into the row. Unlike iceberg, hive's files do not retain the partition column, but the BE still reads it from the row to build the directory, and THiveTableSink has no static-partition field to carry it out of band -- the row is the only channel. The SPI javadoc said "data files physically retain partition columns"; broadened to name both reasons a connector lands here. 2. HiveConnectorMetadata.validateStaticPartitionColumns stops being a no-op and rejects an unknown partition column / an unpartitioned table, case-insensitively (HMS lowercases hive names; Locale.ROOT keeps the fold locale-independent). The knowledge and the messages stay in the connector per the SPI contract. The wording is upstream's hive wording, deliberately NOT iceberg's -- iceberg says "is not partitioned", hive says "is not a partitioned table", and the e2e substring-matches the latter. 3. BindSink resolves static-partition names through table.getColumn() before using them, and rejects a column named twice or named in BOTH the PARTITION clause and the insert column list. getColumn() is already equalsIgnoreCase and is what the two sibling statements in bindConnectorTableSink use; only the exclusion filter compared raw names, so this removes an inconsistency rather than putting a case rule in the engine. Safe because SchemaCacheValue rejects any schema with two columns differing only by case. A name matching no column passes through verbatim -- on iceberg the PARTITION clause names a partition FIELD (category_bucket), which the connector already validated. The duplicate check matters because columnToOutput is a CASE_INSENSITIVE_ORDER map: PARTITION(dt='1', DT='2') would otherwise silently keep only one value. 4. MaxComputeConnectorMetadata gains a fail-loud static-partition validator. Pure regression guard: maxcompute consumes the spec by EXACT key lookup on the raw user-typed keys, which (3) does not reach, so a case-mismatched PARTITION(DS='x') would have flipped from today's loud arity error to a silently empty PartitionSpec. Matching stays case-SENSITIVE -- ODPS's own case semantics are not verified here, so this preserves fail-loud rather than speculatively making maxcompute accept the name. NOT ported: upstream's fe-core validateHiveStaticPartition (its knowledge moves into the connector, above) and its blanket toLowerCase fold (replaced by schema resolution, which cannot mangle iceberg partition-field names). Upstream's HiveDDLAndDMLPlanTest additions are not re-homed: this branch deleted that class with the hive migration, fe-core does not depend on fe-connector-hive, and no TestWithFeService harness here wires a plugin-driven catalog; the assertions are covered by the unit tests below instead. Verification: - full reactor `package` (build-cache disabled, checkstyle active): BUILD SUCCESS - 1409 unit tests green: fe-core 119, hive+iceberg 1158, maxcompute 132 - 7 mutations each turn the intended test class RED and no other: materialize->false, hive validator case-sensitive, hive validator no-op, maxcompute validator case-insensitive, drop canonicalization, drop the duplicate check, drop the insert-column-list reject - e2e test_hive_write_static_partition.groovy needs a hive docker env (owner-run) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FQapWkzRN75GAxeQ3Xaw6B
…pache#65991) Captures the one trap worth carrying forward: unifying the per-connector unbound sinks turned upstream's hard error into a silent wrong-partition write for hive, because the shared sink hands every plugin-driven catalog a static-partition spec that the legacy hive path never received. Git reported no conflict there and the tests were green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FQapWkzRN75GAxeQ3Xaw6B
…ing replay Replaying OP_CREATE_CATALOG builds the connector synchronously, on the thread that is starting the FE, and EditLog's fallback turns anything thrown there into System.exit(-1). So a connector that rejects a property an older FE stored without validating -- or a half-installed plugin throwing NoClassDefFoundError -- keeps the whole FE from starting instead of making one catalog unusable. The image path never had this problem because it builds the connector lazily. Catch on the replay arm only and fall through to the degraded registration that already exists for an unclaimed type; the failure resurfaces at first access as an ordinary query error. Interactive CREATE CATALOG still fails loud, because the user is waiting for the error. Catching RuntimeException | Error rather than Exception is deliberate: a half-installed plugin throws NoClassDefFoundError, not a RuntimeException. Covered by Legacy413JournalReplayTest#connectorConstructorThatThrowsMustNotTakeTheFeDown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mework
Adds golden FE metadata fixtures produced by real Doris 4.1.3 bytecode, and
the tests that replay them on this branch. Goal: a catalog created on 4.1.3
must migrate onto the connector SPI and remain usable, through both the image
and the edit-log path.
The fixtures are generated, never hand-written. 4.1.3's wire format has at
least five details a hand-authored blob gets wrong: HTML escaping is on
(jdbc_url persists '=' and '&' escaped), serializeNulls is off (the resource
key vanishes when null), taap is polymorphic ({} when empty, an array of
2-tuples when not), clazz is written first, and the create path rewrites the
property map before it is persisted. The generator, its provenance and the
regeneration recipe live next to the fixtures; output is byte-deterministic.
Fixture families, and why the resource-backed one is not optional: with a
"type" property present, deleting the migration entirely leaves every
assertion green, because getType() just reads what was already on the wire.
Only the resource-backed catalogs -- which persist no "type" and recover it
from logType -- can fail when the migration is removed. Mutation-verified.
Tests:
Legacy413ImageMigrationTest all 20 legacy catalog labels migrate;
engine-owned types (doris) do not; type
recovery incl. the trino-connector hyphen;
comment/lastUpdateTime/taap/jdbc_url fidelity
Legacy413LabelRegistryTest every label 4.1.3 could write still resolves,
compared registry-to-registry rather than
against a hand-maintained list
Legacy413JournalReplayTest edit-log replay, property fidelity through to
the connector, degraded paths
Legacy413CloudImageTest cloud mode; one guard, not a second matrix,
because the cloud and non-cloud modules differ
in exactly one entry
Legacy413ProviderTypeContractTest the recovered type must be a string a REAL
shipped provider answers to -- the two halves
are literals in different modules
fe-core gains test-scope-only dependencies on the connector modules, matching
the existing fe-filesystem-* arrangement, solely so that last test can compare
both halves. Production still loads connectors from plugins/connector/.
Anti-vacuity: five mutations were applied and each turned the predicted test
red -- disabling the type backfill, dropping the TRINO_CONNECTOR case,
deleting a GSON label, reverting the replay guard, and removing a connector
from the shipped set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
morningman
force-pushed
the
master-catalog-spi-413-upgrade-compat-ut
branch
from
July 29, 2026 13:22
790206f to
95b7435
Compare
Contributor
Author
|
run feut |
…vn test` can compile against them fe-connector-hms-hive-shade and fe-connector-paimon-hive-shade carry no sources: their Hive metastore content exists only once maven-shade has run. With both maven-jar-plugin and maven-shade-plugin bound to `package`, any build that stops earlier leaves those modules with nothing but an (empty) target/classes, and that is what the reactor then hands to their consumers. run-fe-ut.sh builds with `mvn test`, which stops before `package`. Once fe-core took test-scope dependencies on the connector impl modules, the FE UT reactor grew from 40 to 59 modules and began pulling fe-connector-hms in, where every org.apache.hadoop.hive.* import failed to resolve (TeamCity FE UT 1009434: "package org.apache.hadoop.hive.metastore.api does not exist", build aborted before fe-core ran a single test). fe-connector-paimon would have failed the same way on HiveConf one module later. Move both executions to process-classes -- the last phase still ahead of test-compile -- keeping maven-jar-plugin declared first so the main artifact is present before shade, which is what the package-phase pin was protecting under a parallel reactor. Cost is ~21s per FE UT run (paimon 17s, hms 4s), since the build cache is disabled at project level for both modules. Verified: the full 59-module `mvn -pl fe-common,fe-core -am test-compile` succeeds and reproducibly fails on fe-connector-hms without this change; both shade jars rebuild identical to the previous ones (same byte size, same 7940 / 55313 entry listings); `package` still runs each shade exactly once and the hive / paimon / iceberg / hudi plugin zips each carry their shade jar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JZp6xJNzwavabuea7bsAP
Contributor
Author
|
run buildall |
…-classes) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JZp6xJNzwavabuea7bsAP
Contributor
Author
|
run buildall |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
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
2 participants
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.
Only for testing #66214