Skip to content

Commit ae2fb5b

Browse files
morningmanclaude
andcommitted
[refactor](fe) Merge fe-connector-api into fe-connector-spi
### What problem does this PR solve? Problem Summary: The connector plugin contract was split across two modules whose names were inverted relative to common usage: `fe-connector-api` held the types a plugin IMPLEMENTS (`Connector`, `ConnectorMetadata` and its Ops sub-interfaces, the scan / write / procedure providers, handles, pushdown and ddl value types) — which is what "SPI" normally names — while `fe-connector-spi` held mostly the engine services a plugin CONSUMES (`ConnectorContext`, `ConnectorStorageContext`, `ConnectorConf`), which is what "API" normally names. Both modules' package-info already documented the inversion and asked readers to "read the content, not the name". The `fe-connector-metastore-api` / `-metastore-spi` pair right next to them follows the opposite (standard) convention, so one directory carried two contradictory naming rules. Swapping the two names does not fix it. A strict split by "who implements" is circular: `ConnectorProvider.create` takes a `ConnectorContext` (spi -> api), while `ConnectorContext.createSiblingConnector` returns a `Connector` (api -> spi). That cycle is exactly why Trino keeps its whole plugin surface in a single `trino-spi` module, and this framework is modelled on Trino. This commit does the same: `fe-connector-api` is merged into `fe-connector-spi`, and `org.apache.doris.connector.api` becomes `org.apache.doris.connector.spi`. The merge is dependency-neutral — every module that depended on `fe-connector-api` (hive, hudi, maxcompute, paimon, trino, fe-core) already depended on `fe-connector-spi` as well. Two consequences worth calling out: - The connector plugin API version is bumped 2.0 -> 3.0. Every type on the contract changed its fully-qualified name, so a plugin built against 2.0 must be refused by `ApiVersionGate` at load time rather than fail later with `NoClassDefFoundError`. Both recorded baselines (`connector-metadata-methods.txt`, `connector-plugin-surface.txt`) and the version pinned in `ConnectorPluginSurfaceTest` are updated in this commit. `ConnectorPluginManagerTest` used to hardcode the compatible major as a literal; it now derives both the stale and the current major from the kernel's declared version, because what that test asserts is the gate (stale refused, current admitted), not which number is current. `ConnectorPluginSurfaceTest` remains the one deliberate speed bump. - `ChildFirstClassLoader.DEFAULT_PARENT_FIRST_PACKAGES` listed only `org.apache.doris.connector.api.`, so `ConnectorProvider` and `ConnectorContext` were not parent-first. After the merge the single `org.apache.doris.connector.spi.` entry covers the whole contract. The merged `package-info.java` keeps the existing Rule 1-7 design rules; Rule 4 now explains why both directions live in one module instead of disclaiming the inverted names. README.md and AGENTS.md are updated accordingly. ### Release note None ### Check List (For Author) - Test: Unit Test - Full FE build passes, including fe-core test compilation. - Every unit test under `org.apache.doris.connector` passes: 314 test classes / 1421 cases, 0 failures, 0 errors, across the 58-module reactor (fe-core included). - Checkstyle passes across the whole fe reactor. - Both architecture gates pass: `check-fe-connector-imports.sh` (plus its self-test) and `check-fe-core-metadata-funnel.sh`. - No regression test run: this is a pure rename/merge with no behavior change. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 6d7992f commit ae2fb5b

610 files changed

Lines changed: 2540 additions & 2670 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.licenserc.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ header:
5151
- "fe/fe-core/src/test/resources/stageUtilTest.txt"
5252
# Recorded method-surface baseline; ConnectorMetadataSurfaceTest reads every
5353
# non-blank line as a method signature, so a header would break the test.
54-
- "fe/fe-connector/fe-connector-api/src/test/resources/connector-metadata-methods.txt"
54+
- "fe/fe-connector/fe-connector-spi/src/test/resources/connector-metadata-methods.txt"
5555
# Recorded plugin API surface baselines, one per plugin family. Same reason as
5656
# above: each *PluginSurfaceTest.readBaseline() adds every non-blank line to the
5757
# expected signature set, so a header would be read back as phantom signatures.

build-support/check-fe-connector-imports.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
# datasource / qe / analysis / nereids / planner / persist / transaction / fs /
2626
# statistics / mysql / service). Anything they need from fe-core has to be
2727
# exposed through the SPI in
28-
# org.apache.doris.connector.{api,spi,extension,...}
28+
# org.apache.doris.connector.{spi,metastore,...}
2929
# or shared types in org.apache.doris.thrift / org.apache.doris.filesystem.
3030
#
3131
# The gate matches both plain and `import static` imports, scans src/main/java

build-support/tests/test-fe-connector-imports.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import org.apache.doris.mysql.privilege.Auth;
5858
import org.apache.doris.service.FrontendOptions;
5959
import org.apache.doris.thrift.TFoo;
6060
import org.apache.doris.filesystem.Bar;
61-
import org.apache.doris.connector.api.Baz;
61+
import org.apache.doris.connector.spi.Baz;
6262
import org.apache.doris.datasource.hive.HiveVersionUtil;
6363
import static org.apache.doris.datasource.hive.HiveVersionUtil.SOME_CONST;
6464
public class FakeConn {}
@@ -117,7 +117,7 @@ must_not_report() {
117117
}
118118
must_not_report 'org.apache.doris.thrift.TFoo' # SPI-shared
119119
must_not_report 'org.apache.doris.filesystem.Bar' # SPI-shared
120-
must_not_report 'org.apache.doris.connector.api.Baz' # SPI
120+
must_not_report 'org.apache.doris.connector.spi.Baz' # SPI
121121
must_not_report 'HiveVersionUtil;' # vendored (non-static)
122122
must_not_report 'HiveVersionUtil.SOME_CONST;' # vendored (static) — E3
123123

build.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -723,8 +723,7 @@ if [[ "${BUILD_FE}" -eq 1 ]]; then
723723
fi
724724
done
725725
unset _fs_mod
726-
# Connector API, SPI, and plugin modules (loaded at runtime as plugins)
727-
modules+=("fe-connector/fe-connector-api")
726+
# Connector SPI and plugin modules (loaded at runtime as plugins)
728727
modules+=("fe-connector/fe-connector-spi")
729728
for _conn_mod in es jdbc maxcompute trino hms hive paimon hudi iceberg; do
730729
if [[ -d "${DORIS_HOME}/fe/fe-connector/fe-connector-${_conn_mod}" ]]; then

fe/fe-connector/AGENTS.md

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ mvn -f fe/pom.xml -pl :fe-connector-<module> -am test
1414

1515
# Run a single test class. Keep -DfailIfNoTests=false: with -am, upstream
1616
# modules have no matching tests and would fail the build otherwise.
17-
mvn -f fe/pom.xml -pl :fe-connector-api -am test \
17+
mvn -f fe/pom.xml -pl :fe-connector-spi -am test \
1818
-Dtest=ConnectorMetadataSurfaceTest -DfailIfNoTests=false
1919
```
2020

@@ -40,20 +40,25 @@ self-tests live in `build-support/` and `build-support/tests/`):
4040
wired into this directory's `pom.xml`. fe-connector modules must not
4141
import fe-core internals, in main OR test sources. When a connector needs
4242
something from the engine, the fix is to extend the SPI in
43-
fe-connector-api / fe-connector-spi — never to import fe-core.
43+
fe-connector-spi — never to import fe-core.
4444
2. **Metadata funnel**`build-support/check-fe-core-metadata-funnel.sh`,
4545
wired into fe-core's `pom.xml`. Inside fe-core, only
4646
`PluginDrivenMetadata` may call `Connector#getMetadata`; exempt call
4747
sites carry a `getMetadata-funnel-exempt` marker, and deleting a marker
4848
auto-tightens the gate.
4949

50-
**Changing the shared SPI surface (fe-connector-api):** regenerate
51-
`fe-connector-api/src/test/resources/connector-metadata-methods.txt` in the
52-
SAME commit — run `ConnectorMetadataSurfaceTest` and copy the "actual" set
53-
from the failure output. Always run fe-connector-api's OWN suite after an
54-
SPI change; running only consumer modules will not catch a stale baseline.
55-
The same suite pins the `@ConnectorMustImplement` set, so promoting a method
56-
into the minimum implementation set is a deliberate, reviewed change.
50+
**Changing the shared SPI surface (fe-connector-spi):** regenerate BOTH
51+
recorded baselines in the SAME commit — `connector-metadata-methods.txt`
52+
(`ConnectorMetadataSurfaceTest`) and `connector-plugin-surface.txt`
53+
(`ConnectorPluginSurfaceTest`), under
54+
`fe-connector-spi/src/test/resources/`; run the test and copy the "actual"
55+
set from the failure output. Any surface change is a MAJOR change: bump
56+
`connector.plugin.api.version` in `fe/fe-connector/pom.xml` and the version
57+
pinned in `ConnectorPluginSurfaceTest` in that same commit. Always run
58+
fe-connector-spi's OWN suite after an SPI change; running only consumer
59+
modules will not catch a stale baseline. The same suite pins the
60+
`@ConnectorMustImplement` set, so promoting a method into the minimum
61+
implementation set is a deliberate, reviewed change.
5762

5863
## Invariants Without a Gate
5964

@@ -94,8 +99,9 @@ these has a concrete failure mode.
9499

95100
## Task Recipes
96101

97-
- **Change the SPI surface**: edit fe-connector-api → regenerate the
98-
baseline in the same commit → run fe-connector-api's suite → adjust
102+
- **Change the SPI surface**: edit fe-connector-spi → regenerate both
103+
baselines and bump the plugin API version in the same commit → run
104+
fe-connector-spi's suite → adjust
99105
affected connectors (grep for overrides) → run their module tests.
100106
- **Fix a connector bug**: module-scoped build + tests (recipes above;
101107
paimon via `install`). For user-visible behavior, extend the e2e suites

fe/fe-connector/README.md

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ Roles only — one line each. For anything deeper, read the module's javadoc
4040

4141
| Module | Role |
4242
|---|---|
43-
| `fe-connector-api` | The engine <-> connector contract: `ConnectorMetadata` with its Ops sub-interfaces, plus handle / pushdown / mvcc / scan / write / ddl / procedure / event / rest types. The javadoc here **is** the API reference. |
44-
| `fe-connector-spi` | Bootstrap contract: `ConnectorProvider` (discovery identity + factory) and `ConnectorContext` (what the engine hands a connector, including sibling-connector creation). |
43+
| `fe-connector-spi` | The whole engine <-> connector contract, in both directions. What a connector implements: `ConnectorProvider` (discovery identity + factory), `Connector`, `ConnectorMetadata` with its Ops sub-interfaces, plus handle / pushdown / mvcc / scan / write / ddl / procedure / event / rest types. What the engine implements and hands down: `ConnectorContext` (including sibling-connector creation), `ConnectorStorageContext`, `ConnectorSession`, `ConnectorConf`. The javadoc here **is** the API reference. |
44+
45+
The two directions are one module on purpose: the boundary is bidirectional
46+
(`ConnectorProvider.create` takes a `ConnectorContext`, and `ConnectorContext`
47+
hands back a `Connector`), so splitting it by "who implements" would be
48+
circular. Trino makes the same call with `trino-spi`. Contrast the metastore
49+
layer below, where the split is acyclic and the usual api/spi convention holds.
4550

4651
**Metastore layer** (how connectors reach a metastore without hand-parsing
4752
endpoint properties)
@@ -99,8 +104,8 @@ exactly one `ConnectorMetadata` instance per catalog on the statement's
99104
`ConnectorStatementScope` and closes it deterministically at statement end.
100105
Scan planning follows the same shape: the generic `PluginDrivenScanNode`
101106
(fe-core) delegates all per-source planning to the connector's
102-
`ConnectorScanPlanProvider` — note the interface lives in fe-connector-api
103-
(`api.scan`), not in fe-core.
107+
`ConnectorScanPlanProvider` — note the interface lives in fe-connector-spi
108+
(`spi.scan`), not in fe-core.
104109

105110
**Classloading.** Plugins load child-first, each carrying its own runtime
106111
closure. Wherever engine code crosses into a plugin — or a bundled library
@@ -119,17 +124,16 @@ This document never lists SPI methods. The truth lives in code, behind four
119124
mechanisms:
120125

121126
1. **Javadoc is the API reference.** Start at `ConnectorMetadata` (and its
122-
Ops sub-interfaces) in fe-connector-api, and `ConnectorProvider` in
123-
fe-connector-spi. Every SPI method has a default body, so each
124-
sub-interface's class javadoc states its minimum implementation set,
125-
lifecycle, and threading rules.
127+
Ops sub-interfaces) and `ConnectorProvider`, both in fe-connector-spi.
128+
Every SPI method has a default body, so each sub-interface's class javadoc
129+
states its minimum implementation set, lifecycle, and threading rules.
126130
2. **`@ConnectorMustImplement`** is the machine-readable half of the minimum
127131
implementation set: it marks the default methods a connector is
128132
nevertheless expected to override, with `when` naming the capability that
129133
triggers the obligation. A unit test pins the annotated set, so promoting
130134
a method is a deliberate, reviewed change.
131135
3. **The recorded surface.**
132-
`fe-connector-api/src/test/resources/connector-metadata-methods.txt`
136+
`fe-connector-spi/src/test/resources/connector-metadata-methods.txt`
133137
freezes the public method surface of `ConnectorMetadata`;
134138
`ConnectorMetadataSurfaceTest` fails on any drift. Adding, removing, or
135139
moving SPI methods must regenerate this baseline in the same commit (run
@@ -146,7 +150,7 @@ mechanisms:
146150
source; if a connector does not opt in, the feature stays off.
147151

148152
Statement-scoped memoization has one framework-wide convention worth knowing
149-
before you read connector code: `ConnectorStatementScopes` (fe-connector-api)
153+
before you read connector code: `ConnectorStatementScopes` (fe-connector-spi)
150154
keys per-statement values by `(catalogId, db, table, queryId)` plus a
151155
connector-owned namespace constant prefixed with the connector's
152156
`getType()`. Each connector guards its own prefix with a unit test
@@ -239,10 +243,10 @@ metastore/shade/cache). For a write path, the richest example is
239243
## Testing and Verification
240244

241245
- **Unit tests** live in each module; build/test recipes are in `AGENTS.md`.
242-
- **The shared SPI surface** is guarded by fe-connector-api's own suite
243-
(`ConnectorMetadataSurfaceTest`). Whenever you touch fe-connector-api, run
244-
that module's tests — a consumer-only test run will not catch a stale
245-
baseline.
246+
- **The shared SPI surface** is guarded by fe-connector-spi's own suite
247+
(`ConnectorMetadataSurfaceTest` and `ConnectorPluginSurfaceTest`). Whenever
248+
you touch fe-connector-spi, run that module's tests — a consumer-only test
249+
run will not catch a stale baseline.
246250
- **Architecture gates** run in the `validate` phase of every FE build: the
247251
forbidden-import gate for this directory and the metadata-funnel gate for
248252
fe-core. Scripts and their self-tests live in `build-support/` and

fe/fe-connector/fe-connector-api/pom.xml

Lines changed: 0 additions & 81 deletions
This file was deleted.

0 commit comments

Comments
 (0)