[draft] Master catalog spi 03#64095
Closed
morningman wants to merge 16 commits into
Closed
Conversation
morningman
requested review from
CalvinKirs,
gavinchou and
mymeiyi
as code owners
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29334 ms |
Contributor
TPC-DS: Total hot run time: 169151 ms |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
Author
|
run buildall |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29447 ms |
Contributor
Author
|
run buildall |
Contributor
TPC-DS: Total hot run time: 170249 ms |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
morningman
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29182 ms |
Contributor
TPC-H: Total hot run time: 29243 ms |
Contributor
TPC-DS: Total hot run time: 168942 ms |
Contributor
TPC-DS: Total hot run time: 168867 ms |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29425 ms |
Contributor
TPC-DS: Total hot run time: 168454 ms |
Contributor
FE Regression Coverage ReportIncrement line coverage |
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) ## Summary 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). ## Test plan - [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>
Add P2 task file with 13 tasks across 5 batches (A SPI 补齐 / B fe-core 桥接 / C 翻闸 / D 清旧 / E 测试+文档), refresh PROGRESS / HANDOFF / connectors/trino-connector.md with the post-P1 recon findings (10 .java in fe-core/datasource/trinoconnector, 5 live external callers, 1 reverse instanceof, fe-connector-trino SPI ~95% with gaps in validateProperties, preCreateValidation, pushdown ops) and the two open-question rulings (pushdown stays in P2 batch A; GsonUtils 402/457/476 class-token entries get deleted together with the new string-name redirect). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…or batch A T01 - TrinoConnectorProvider.validateProperties enforces the required trino.connector.name property at CREATE CATALOG time (ported from fe-core TrinoConnectorExternalCatalog.checkProperties). TrinoDorisConnector adds preCreateValidation that calls ensureInitialized() so plugin loading and connector-factory resolution happen at catalog creation rather than on the first SELECT. T02 - TrinoConnectorDorisMetadata implements applyFilter and applyProjection. applyFilter reuses the existing TrinoPredicateConverter to translate the Doris ConnectorExpression into a Trino TupleDomain, invokes Trino native applyFilter, and wraps the returned table handle into a new TrinoTableHandle that retains the original column maps. applyProjection builds Trino Variable projections + column assignments from the SPI ConnectorColumnHandle list, invokes Trino native applyProjection, and returns a ProjectionApplicationResult with column-ref projections and assignments for future SPI consumers. remainingFilter is conservatively returned as the original expression to match legacy fe-core behavior where conjuncts are not stripped and BE re-evaluates them; refining this to map Trino residual TupleDomain back to a ConnectorExpression is left as a follow-up. Strictly fe-connector-trino side; no fe-core changes (batch B will handle GsonUtils redirect, gsonPostProcess logType migration, and engine-name branches). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… for batch B T03 - GsonUtils replaces the three legacy registerSubtype entries for TrinoConnectorExternalCatalog / Database / Table with registerCompatibleSubtype mappings onto PluginDrivenExternalCatalog / Database / Table. The HANDOFF originally framed T03 as "add-only" and deferred deletion to T10, but RuntimeTypeAdapterFactory enforces label uniqueness in both registerSubtype and registerCompatibleSubtype - having both bindings simultaneously throws IllegalArgumentException at static init. ES/JDBC did the equivalent replacement atomically in 5c32565; mirror that pattern here. T10 in batch D no longer needs to touch GsonUtils - just the directory removal. T04 - PluginDrivenExternalCatalog.gsonPostProcess extracts a legacyLogTypeToCatalogType helper. The previous code used logType.name().toLowerCase(Locale.ROOT) generically, which produces "trino_connector" for Type.TRINO_CONNECTOR but CatalogFactory expects "trino-connector" (hyphen). ES/JDBC happen to match lowercase-name by coincidence. The helper maps TRINO_CONNECTOR explicitly and falls back to name().toLowerCase() for the existing connectors. Future connectors with diverging factory keys (e.g., MaxCompute) add cases here. T05 - The original task description was based on a non-existent API (ExternalCatalog.registerCompatibleSubtype). The only registerCompatibleSubtype surface is on RuntimeTypeAdapterFactory in GsonUtils, which T03 already edits. T05 is satisfied by T03 transitively; the task entry is preserved for traceability but marked as redundant. T06 - PluginDrivenExternalTable.getEngine and getEngineTableTypeName each get a case "trino-connector" branch. getEngine returns TRINO_CONNECTOR_EXTERNAL_TABLE.toEngineName() which is null because TableType.toEngineName has no case for TRINO_CONNECTOR_EXTERNAL_TABLE - this matches the pre-migration display where the legacy TrinoConnectorExternalTable also fell through to a null engine string. getEngineTableTypeName returns the enum .name(), preserving the legacy table-type label across SHOW TABLE STATUS and information_schema. Together these enable image deserialization to redirect legacy trino catalogs / databases / tables onto the PluginDrivenExternal* hierarchy with a correctly backfilled "type" property and preserved engine-name display. CatalogFactory still routes case "trino-connector" to the legacy factory until batch C T07 flips SPI_READY_TYPES; the intermediate window between this commit and T07 means newly created trino catalogs will fail to serialize (no registerSubtype anymore), so batch B and C must land back-to-back before any deployment. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Capture the post-batch-B state and lay out the next session step-by-step: batch C T07 SPI_READY flip is a one-line edit but must be followed immediately by batch D so the regression window between the deleted GsonUtils registerSubtype and the still-legacy CatalogFactory routing closes quickly. Record the three HANDOFF description corrections found this session (T03 cannot be add-only, T05 was duplicate of T03, T04 name-to-string mapping needed an explicit helper, T06 returns null per legacy) so the next agent does not retread them. Mark batch A and batch B complete in connectors/trino-connector.md and PROGRESS session status; refresh the commit list with 31fb91c5bd3 and dfd48725c76. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… from fe-core - T08 PhysicalPlanTranslator: drop instanceof TrinoConnectorExternalTable scan branch (PluginDrivenExternalTable SPI branch already handles trino-connector) - T09 CatalogFactory: drop legacy case "trino-connector" (SPI_READY_TYPES routes it now) - T10 remove datasource/trinoconnector/ (10 files) + legacy TrinoConnectorPredicateTest - ExternalCatalog: TRINO_CONNECTOR db-build case -> PluginDrivenExternalDatabase (mirror JDBC) Retain image-compat enums (InitCatalogLog.Type.TRINO_CONNECTOR, TableType.TRINO_CONNECTOR_EXTERNAL_TABLE), GsonUtils label redirects, and the MetastoreProperties trino-connector entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cover the plugin-free converters of the Trino connector SPI bridge: - 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 fails fast when the required trino.connector.name property is missing/empty (T01) 29 tests, JUnit 5; no Trino plugin/cluster needed. Plugin-dependent paths remain covered by the external_table_p0/p2 trino_connector regression suites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Flip P2 trino-connector tracking to code-complete (batches C/D/E): - tasks/P2 + PROGRESS: T07-T11,T13 ✅, T12 🟡 deferred; refresh stale post-rebase hashes; add 2026-06-04 phase/progress-log entries - connectors/trino-connector.md: 100%; playbook steps 7/10/11/13 ✅ - deviations-log: record DV-001 (batch D missed ExternalCatalog db-route + legacy test), DV-002 (T11 no-mock / json+schema dropped), DV-003 (T12 deferred), DV-004 (docs-next lives in website repo) - HANDOFF: rewrite for P2-complete; PR pending branch-base alignment (catalog-spi-03 on master vs branch-catalog-spi at P1); note the rebase stale-DorisParser / clean-fe-core build gotcha Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…torContext environment The trino-connector plugin runs in an isolated classloader (ConnectorPluginManager makes only org.apache.doris.connector/filesystem parent-first), so it loads its own bundled org.apache.doris.common.Config with default values and never sees fe.conf. Reading Config.trino_connector_plugin_dir inside the plugin always resolved to the default DORIS_HOME/plugins/connectors, so the FE scanned an empty dir and every trino-connector catalog failed with 'Cannot find Trino ConnectorFactory'. Publish trino_connector_plugin_dir through DefaultConnectorContext.getEnvironment() (read with fe-core's real Config) and resolve it in the plugin from the environment map, mirroring how the JDBC connector receives jdbc_drivers_dir. TrinoScanPlanProvider now reuses the initialized bootstrap singleton instead of re-resolving. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e resolves
The BE JNI reader re-adds the 'trino.' prefix to every entry of trinoConnectorOptions (TRINO_CONNECTOR_OPTION_PREFIX), then the BE Java scanner strips one 'trino.' and reads connector.name from the result. TrinoScanPlanProvider.serializeOptions was sending session.getCatalogProperties() (the full, trino.-prefixed catalog properties), so the key arrived as trino.connector.name and BE's remove("connector.name") returned null, throwing 'NullPointerException: name is null' in ConnectorName.<init> on query.
Send the stripped trino.* properties (dorisConnector.getTrinoProperties(), which includes connector.name), matching the legacy TrinoConnectorScanNode, and preserve the catalog's create_time (part of BE's connector cache key).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ordering
Two regressions from the catalog-spi trino connector rewrite, both caught by external_table_p0/trino_connector regression tests:
1. desc <table> reported Key=false for every column because TrinoConnectorDorisMetadata built ConnectorColumn without isKey. External trino tables follow the Doris convention of marking all columns as key, so pass isKey=true (matches legacy TrinoConnectorExternalTable / JdbcClient). Fixes the desc_all_types_null mismatch in trino_{pg,mysql,clickhouse,sqlserver,oracle,kafka_base}.
2. Hive scans failed with JNI ArrayIndexOutOfBoundsException ("Index N out of bounds for length N, cur path: connector://virtual") because the column handles/metadata serialized to BE used the full, HashMap-unordered column map instead of the projected columns in projection order. Use the projected handle returned by applyProjection, serialize only the projected columns in order, and use LinkedHashMap for projection assignments so Trino getRecordSet() handle-vs-columns verify passes. Fixes trino_hive_{parquet,orc,other,schema_evolution,tablesample_p0,serde_prop,different_parquet_types,prepare_hive_data_in_case}.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…om fe-core Legacy trino-connector code under fe-core was removed in T08-T10, but the io.trino:trino-main compile dependency in fe-core/pom.xml was left behind. fe-core no longer references any io.trino.* class (zero imports across main and test sources), so it was a dead declared dependency. Verified: clean fe-core + 'mvn -pl fe-core -am test-compile' recompiles 4658 main + 1222 test sources against the trino-less classpath, BUILD SUCCESS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
morningman
force-pushed
the
master-catalog-spi-03
branch
from
6bde562 to
c4ac2c5
Compare
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
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