diff --git a/.gitleaks.toml b/.gitleaks.toml index 8fb5f5626121aa..c01003b5a6c548 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -6,6 +6,12 @@ useDefault = true [[rules]] id = "generic-api-key" +[[rules.allowlists]] +description = "Ignore prose 'version token: iceberg/paimon' in code comments/plan-doc — a cache version-token discussion, not a credential (generic-api-key false positive: 'iceberg/paimon' entropy 3.52 barely clears the 3.5 threshold)" +condition = "AND" +regexTarget = "match" +regexes = ['''token:\s*iceberg/paimon'''] + [[rules.allowlists]] description = "Ignore the fake OIDC token fixture in AuthenticatorManagerTest" condition = "AND" diff --git a/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java new file mode 100644 index 00000000000000..cd7f03be986ee7 --- /dev/null +++ b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java @@ -0,0 +1,110 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.cache; + +import java.util.Collections; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ForkJoinPool; +import java.util.function.Supplier; + +/** + * GENERIC (engine-agnostic) cross-query cache of a connector's derived partition view — "cache A" of the + * external-partition-derived-cache design (design doc {@code 2026-07-20-external-partition-derived-cache-design.md} + * §5). It is the generic version of {@code org.apache.doris.connector.iceberg.IcebergPartitionCache}: same + * construction pattern (a contextual-only, manual-miss-load {@link MetaCacheEntry}), same {@link CacheSpec} wiring, + * same invalidation style — but keyed by the engine-agnostic {@link PartitionViewCacheKey} and holding an opaque + * value {@code V} instead of an iceberg-specific raw-partition list, so it has no engine-specific imports and can be + * shared by every connector (iceberg/paimon/hive/maxcompute, wired in later subsystems — this class has NO + * consumers yet). + * + *

Config: {@code meta.cache..partition_view.(enable|ttl-second|capacity)}, default ON / 86400s / + * 1000 entries (matching {@code IcebergPartitionCache}'s {@code DEFAULT_TABLE_CACHE_CAPACITY}). {@code enable=false} + * / {@code ttl-second=0} / {@code capacity=0} each disable the cache (see {@link CacheSpec#isCacheEnabled}): {@link + * #get} then calls the loader on every call, matching {@code IcebergPartitionCache}'s disabled-cache bypass. + * + *

Concurrency: mirrors {@code IcebergPartitionCache} / {@code MaxComputePartitionCache} exactly — the + * entry is contextual-only (no built-in loader; the caller supplies one per {@link #get} call) with manual miss + * load, so a slow remote enumeration runs OUTSIDE Caffeine's compute lock (deduplicated per key by a striped lock) + * on the calling thread, and its exception propagates to the caller unwrapped without poisoning the cache. + */ +public final class ConnectorPartitionViewCache { + + /** {@code meta.cache..partition_view.*} — the property-namespace entry name for this cache. */ + static final String ENTRY_PARTITION_VIEW = "partition_view"; + /** Default TTL: 24h, matching the design doc's stated default and sibling caches' 24h TTL. */ + static final long DEFAULT_TTL_SECOND = 86400L; + /** Default capacity, matching {@code IcebergPartitionCache}'s {@code DEFAULT_TABLE_CACHE_CAPACITY}. */ + static final long DEFAULT_CAPACITY = 1000L; + + private final MetaCacheEntry entry; + + /** + * @param engine engine token for the {@code meta.cache..partition_view.*} property namespace, e.g. + * {@code "iceberg"}/{@code "paimon"}/{@code "hive"}/{@code "max_compute"}. + * @param props the catalog properties; drives the {@link CacheSpec} (enable/ttl-second/capacity). May be + * {@code null}, treated as empty (defaults apply). + */ + public ConnectorPartitionViewCache(String engine, Map props) { + Objects.requireNonNull(engine, "engine can not be null"); + Map properties = props == null ? Collections.emptyMap() : props; + CacheSpec spec = CacheSpec.fromProperties(properties, engine, ENTRY_PARTITION_VIEW, + CacheSpec.of(true, DEFAULT_TTL_SECOND, DEFAULT_CAPACITY)); + // contextual-only (loader == null, supplied per-call by get()) + manual-miss-load, no auto-refresh -- + // identical shape to IcebergPartitionCache / MaxComputePartitionCache's entry construction. + this.entry = new MetaCacheEntry<>(engine + "." + ENTRY_PARTITION_VIEW, null, spec, + ForkJoinPool.commonPool(), false, true, 0L, true); + } + + /** Caching is on only when the resolved {@link CacheSpec} is effectively enabled (see {@link #entry}'s spec). */ + public boolean isEnabled() { + return entry.stats().isEffectiveEnabled(); + } + + /** + * Returns the cached value for {@code key} if present, else runs {@code loader}, caches and returns it. + * Disabled cache -> {@code loader} runs on every call. The loader runs OUTSIDE Caffeine's compute lock + * (single-flight per key) and its exception propagates unwrapped; a failed load is never cached. + */ + public V get(PartitionViewCacheKey key, Supplier loader) { + Objects.requireNonNull(loader, "loader can not be null"); + return entry.get(key, ignored -> loader.get()); + } + + /** Evicts every cached snapshot/schema entry of one (db, table). Backs REFRESH TABLE / table-level invalidation. */ + public void invalidateTable(String db, String table) { + entry.invalidateIf(key -> key.matches(db, table)); + } + + /** Evicts every cached entry of one db (all its tables). Backs REFRESH DATABASE / db-level invalidation. */ + public void invalidateDb(String db) { + entry.invalidateIf(key -> key.matchesDb(db)); + } + + /** Evicts every cached entry. Backs REFRESH CATALOG. */ + public void invalidateAll() { + entry.invalidateAll(); + } + + /** Test-only: current number of cached entries (accurate map membership, not Caffeine's estimate). */ + long size() { + int[] count = {0}; + entry.forEach((key, value) -> count[0]++); + return count[0]; + } +} diff --git a/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java new file mode 100644 index 00000000000000..f00d39465475a6 --- /dev/null +++ b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java @@ -0,0 +1,100 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.cache; + +import java.util.Objects; + +/** + * Immutable cache key for {@link ConnectorPartitionViewCache}: {@code (db, table, snapshotId, schemaId)}. + * + *

Engine-agnostic (external-partition-derived-cache design doc §5, "cache A"): a table's derived partition + * view is a pure function of its identity plus the MVCC coordinate it was read at, so pinning that coordinate + * into the key is what makes the cache "always correct" — a new snapshot/schema yields a new key, never a stale + * hit. Non-MVCC engines (hive) or engines without a separate schema version pass {@code snapshotId = -1} / + * {@code schemaId = -1}; the key still holds them, it just means "unversioned" for that axis. + * + *

{@link #matches} / {@link #matchesDb} back {@link ConnectorPartitionViewCache#invalidateTable} / + * {@link ConnectorPartitionViewCache#invalidateDb}, which must drop every snapshot/schema of a (db, table) or + * every table of a db — mirrors the {@code matches}/{@code matchesDb} helpers on the sibling connector caches + * ({@code MaxComputePartitionCache.PartitionKey}, {@code HiveFileListingCache.FileListingKey}). + */ +public final class PartitionViewCacheKey { + private final String db; + private final String table; + private final long snapshotId; + private final long schemaId; + + public PartitionViewCacheKey(String db, String table, long snapshotId, long schemaId) { + this.db = db; + this.table = table; + this.snapshotId = snapshotId; + this.schemaId = schemaId; + } + + public String getDb() { + return db; + } + + public String getTable() { + return table; + } + + public long getSnapshotId() { + return snapshotId; + } + + public long getSchemaId() { + return schemaId; + } + + /** Whether this key belongs to the given (db, table), regardless of snapshotId/schemaId. */ + public boolean matches(String db, String table) { + return Objects.equals(this.db, db) && Objects.equals(this.table, table); + } + + /** Whether this key belongs to the given db, regardless of table/snapshotId/schemaId. */ + public boolean matchesDb(String db) { + return Objects.equals(this.db, db); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof PartitionViewCacheKey)) { + return false; + } + PartitionViewCacheKey that = (PartitionViewCacheKey) o; + return snapshotId == that.snapshotId + && schemaId == that.schemaId + && Objects.equals(db, that.db) + && Objects.equals(table, that.table); + } + + @Override + public int hashCode() { + return Objects.hash(db, table, snapshotId, schemaId); + } + + @Override + public String toString() { + return "PartitionViewCacheKey{db=" + db + ", table=" + table + + ", snapshotId=" + snapshotId + ", schemaId=" + schemaId + '}'; + } +} diff --git a/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java new file mode 100644 index 00000000000000..95a80e5d6f164d --- /dev/null +++ b/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java @@ -0,0 +1,278 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.cache; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Unit tests for {@link ConnectorPartitionViewCache} — the GENERIC (engine-agnostic) cache A of the + * external-partition-derived-cache design (design doc {@code 2026-07-20-external-partition-derived-cache-design.md} + * §5). Mirrors {@link org.apache.doris.connector.iceberg.IcebergPartitionCache}'s test shape (that class is the + * iceberg-specific ancestor this generic framework is modeled on), but with a generic string value type ({@code V}) + * since this module must not depend on any engine-specific type. Covers: miss-then-hit, distinct-snapshot keying, + * per-table / per-db / whole-cache invalidation, and the disabled-cache bypass (both {@code enable=false} and + * {@code ttl-second=0}). + */ +public class ConnectorPartitionViewCacheTest { + + private static final String ENGINE = "testengine"; + + private static PartitionViewCacheKey key(String db, String table, long snapshotId, long schemaId) { + return new PartitionViewCacheKey(db, table, snapshotId, schemaId); + } + + private static ConnectorPartitionViewCache newCache() { + return new ConnectorPartitionViewCache<>(ENGINE, new HashMap<>()); + } + + @Test + public void missThenHitLoaderRunsOnceWithinTtl() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + String first = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "view-v1"; + }); + // Second read of the SAME key must be served from cache, NOT re-invoke the loader -- this is the whole + // point of memoizing the expensive partition enumeration across queries. MUTATION: always calling the + // loader -> second != first / loads == 2 -> red. + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "view-v2"; + }); + + Assertions.assertEquals("view-v1", first); + Assertions.assertEquals("view-v1", second, "within TTL the cached value must be served, not a fresh load"); + Assertions.assertEquals(1, loads.get(), "the loader must run exactly once for the same key"); + Assertions.assertTrue(cache.isEnabled()); + } + + @Test + public void differentSnapshotIdIsADistinctEntry() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db", "t", 1L, 1L), () -> { + loads.incrementAndGet(); + return "snap-1"; + }); + // A new commit yields a new snapshotId -> a distinct key -> the loader must run again. MUTATION: keying + // by (db, table) only (ignoring snapshotId/schemaId) -> loads stays 1 / serves the stale snap-1 value. + String second = cache.get(key("db", "t", 2L, 1L), () -> { + loads.incrementAndGet(); + return "snap-2"; + }); + + Assertions.assertEquals("snap-2", second); + Assertions.assertEquals(2, loads.get(), "distinct snapshotId must trigger a distinct load"); + } + + @Test + public void differentSchemaIdIsADistinctEntry() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db", "t", 1L, 1L), () -> { + loads.incrementAndGet(); + return "schema-1"; + }); + // schemaId is part of the key too (paimon evolves schema independently of snapshot). MUTATION: dropping + // schemaId from equals/hashCode -> this second call would incorrectly hit -> loads stays 1. + String second = cache.get(key("db", "t", 1L, 2L), () -> { + loads.incrementAndGet(); + return "schema-2"; + }); + + Assertions.assertEquals("schema-2", second); + Assertions.assertEquals(2, loads.get(), "distinct schemaId must trigger a distinct load"); + } + + @Test + public void invalidateTableEvictsAllSnapshotsOfThatTableOnly() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db", "t", 1L, 1L), () -> "v1"); + cache.get(key("db", "t", 2L, 1L), () -> "v2"); + cache.get(key("db", "other", 1L, 1L), () -> "v3"); + + // REFRESH TABLE db.t must drop BOTH snapshot entries of db.t and leave db.other intact. MUTATION: + // invalidating a single (db, table, snapshotId) key instead of every snapshot of the table -> the + // reload below would still hit the stale "v1". + cache.invalidateTable("db", "t"); + + String reload = cache.get(key("db", "t", 1L, 1L), () -> { + loads.incrementAndGet(); + return "v1-reloaded"; + }); + Assertions.assertEquals("v1-reloaded", reload, "invalidateTable must force a fresh load for db.t"); + Assertions.assertEquals(1, loads.get()); + + // db.other must be untouched by the db.t invalidation. + AtomicInteger otherLoads = new AtomicInteger(); + String other = cache.get(key("db", "other", 1L, 1L), () -> { + otherLoads.incrementAndGet(); + return "v3-should-not-reload"; + }); + Assertions.assertEquals("v3", other, "invalidateTable(db, t) must not touch db.other"); + Assertions.assertEquals(0, otherLoads.get()); + } + + @Test + public void invalidateDbClearsOnlyThatDbsTables() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db1", "t1", 1L, 1L), () -> "a"); + cache.get(key("db1", "t2", 1L, 1L), () -> "b"); + cache.get(key("db2", "t1", 1L, 1L), () -> "c"); + + // REFRESH DATABASE db1 must drop every table of db1 and leave db2 intact. MUTATION: invalidateDb acting + // like invalidateAll (or a no-op) -> either db2 also reloads, or db1 does not. + cache.invalidateDb("db1"); + + String reloadedT1 = cache.get(key("db1", "t1", 1L, 1L), () -> { + loads.incrementAndGet(); + return "a-reloaded"; + }); + Assertions.assertEquals("a-reloaded", reloadedT1); + Assertions.assertEquals(1, loads.get()); + + AtomicInteger db2Loads = new AtomicInteger(); + String db2Value = cache.get(key("db2", "t1", 1L, 1L), () -> { + db2Loads.incrementAndGet(); + return "c-should-not-reload"; + }); + Assertions.assertEquals("c", db2Value, "invalidateDb(db1) must not touch db2"); + Assertions.assertEquals(0, db2Loads.get()); + } + + @Test + public void invalidateAllClearsEveryEntry() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db1", "t1", 1L, 1L), () -> "a"); + cache.get(key("db2", "t2", 1L, 1L), () -> "b"); + + cache.invalidateAll(); + + String reloaded = cache.get(key("db1", "t1", 1L, 1L), () -> { + loads.incrementAndGet(); + return "a-reloaded"; + }); + Assertions.assertEquals("a-reloaded", reloaded, "invalidateAll must drop every entry"); + Assertions.assertEquals(1, loads.get()); + } + + @Test + public void enableFalseDisablesCacheAlwaysLoads() { + AtomicInteger loads = new AtomicInteger(); + Map props = new HashMap<>(); + props.put("meta.cache." + ENGINE + ".partition_view.enable", "false"); + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, props); + + String first = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v1"; + }); + // enable=false must bypass the cache entirely -- every call re-invokes the loader. MUTATION: ignoring the + // enable=false property -> second call would return "v1" (cached) / loads would stay 1 -> red. + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v2"; + }); + + Assertions.assertEquals("v1", first); + Assertions.assertEquals("v2", second, "enable=false must call the loader on every get"); + Assertions.assertEquals(2, loads.get()); + Assertions.assertFalse(cache.isEnabled()); + } + + @Test + public void ttlZeroDisablesCacheAlwaysLoads() { + AtomicInteger loads = new AtomicInteger(); + Map props = new HashMap<>(); + props.put("meta.cache." + ENGINE + ".partition_view.ttl-second", "0"); + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, props); + + cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v1"; + }); + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v2"; + }); + + // ttl-second=0 is CacheSpec's explicit disable signal (distinct from -1 = "no expiration"). MUTATION: + // treating ttl=0 as "no expiration" instead of "disabled" -> second call would return the cached "v1". + Assertions.assertEquals("v2", second, "ttl-second=0 must always load live"); + Assertions.assertEquals(2, loads.get()); + Assertions.assertFalse(cache.isEnabled()); + } + + @Test + public void capacityZeroDisablesCacheAlwaysLoads() { + AtomicInteger loads = new AtomicInteger(); + Map props = new HashMap<>(); + props.put("meta.cache." + ENGINE + ".partition_view.capacity", "0"); + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, props); + + cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v1"; + }); + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v2"; + }); + + Assertions.assertEquals("v2", second, "capacity=0 must always load live"); + Assertions.assertEquals(2, loads.get()); + Assertions.assertFalse(cache.isEnabled()); + } + + @Test + public void defaultsAreEnabledWithNoProperties() { + // No meta.cache..partition_view.* properties set at all -> the built-in default (TTL 86400s, + // capacity 1000, ON) applies, matching IcebergPartitionCache's DEFAULT_TABLE_CACHE_CAPACITY. MUTATION: + // defaulting to disabled -> isEnabled() would be false here. + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, new HashMap<>()); + Assertions.assertTrue(cache.isEnabled(), "the cache must be ON by default with no override properties"); + } + + @Test + public void loaderExceptionPropagatesUnwrappedAndIsNotCached() { + ConnectorPartitionViewCache cache = newCache(); + // A failed load (e.g. a remote enumeration RPC failure) must propagate to the caller verbatim and must + // NOT poison the cache -- a transient failure should not make every subsequent query fail for the TTL. + Assertions.assertThrows(IllegalStateException.class, () -> cache.get(key("db", "t", 5L, 1L), () -> { + throw new IllegalStateException("boom"); + })); + + String recovered = cache.get(key("db", "t", 5L, 1L), () -> "recovered"); + Assertions.assertEquals("recovered", recovered, "a failed load must not be cached"); + } +} diff --git a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java index 1ba13db82c1b64..b11a66c3d8e348 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java +++ b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java @@ -20,6 +20,7 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorCapability; import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorPartitionInfo; import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorTestResult; import org.apache.doris.connector.api.DorisConnectorException; @@ -28,6 +29,7 @@ import org.apache.doris.connector.api.procedure.ConnectorProcedureOps; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; import org.apache.doris.connector.api.write.ConnectorWritePlanProvider; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; import org.apache.doris.connector.hms.CachingHmsClient; import org.apache.doris.connector.hms.HmsClient; import org.apache.doris.connector.hms.HmsClientConfig; @@ -96,6 +98,19 @@ public class HiveConnector implements Connector { // enters SPI_READY_TYPES. Its metastore-metadata sibling is the CachingHmsClient wrapping the HmsClient. private final HiveFileListingCache fileListingCache; + // PERF-06 (S6): cross-query DERIVED partition-view cache ("cache A", the generic ConnectorPartitionViewCache + // from fe-connector-cache), layered ABOVE the raw per-name HMS listing served by CachingHmsClient: it + // memoizes the BUILT List (HiveConnectorMetadata#listPartitionsUncached's per-name + // HiveWriteUtils.toPartitionValues parse + ConnectorPartitionInfo construction), keyed by + // (db, table, -1, -1) — hive is snapshot-less (beginQuerySnapshot always pins -1) and its handle carries no + // schema version, so every query for a table shares ONE entry, invalidated by the hooks below + TTL. ONE + // typed field (like paimon): hive's getMvccPartitionView returns the SPI default (Optional.empty()) for a + // real hive handle, so listPartitions is the only enumeration hook to wrap. Unlike iceberg, hive has NO + // session=user / per-user credential-isolation cache-disabling convention (its per-catalog caches — + // CachingHmsClient, HiveFileListingCache — are already built unconditionally below), so this is constructed + // unconditionally too. + private final ConnectorPartitionViewCache> partitionViewCache; + // Embedded iceberg SIBLING connector: a flipped hms gateway delegates its iceberg-on-HMS tables to it. Built // once per gateway connector (lazily) in the iceberg plugin's OWN child-first classloader via // context.createSiblingConnector — never co-packaged into the hive zip (a second AWS SDK would poison S3 @@ -116,6 +131,9 @@ public HiveConnector(Map properties, ConnectorContext context) { this.properties = Collections.unmodifiableMap(properties); this.context = context; this.fileListingCache = new HiveFileListingCache(this.properties); + // Reads its own meta.cache.hive.partition_view.(enable|ttl-second|capacity) from the catalog properties + // via the framework's CacheSpec (default ON / 24h / 1000). + this.partitionViewCache = new ConnectorPartitionViewCache<>("hive", this.properties); } @Override @@ -170,7 +188,7 @@ private static String rootCauseMessage(Throwable t) { HiveConnectorMetadata newMetadata(HmsClient client) { return new HiveConnectorMetadata(client, properties, context, this::getOrCreateIcebergSibling, this::getOrCreateHudiSibling, this::resolveSiblingOwnerLabeled, - fileListingCache); + fileListingCache, partitionViewCache); } /** @@ -351,6 +369,9 @@ void invalidateTable(HmsClient client, String dbName, String tableName) { ((CachingHmsClient) client).flush(dbName, tableName); } fileListingCache.invalidateTable(dbName, tableName); + // PERF-06: also drop this table's cached derived partition-view entry, so the next listPartitions + // re-derives live. + partitionViewCache.invalidateTable(dbName, tableName); } /** @@ -373,6 +394,7 @@ void invalidateDb(HmsClient client, String dbName) { ((CachingHmsClient) client).flushDb(dbName); } fileListingCache.invalidateDb(dbName); + partitionViewCache.invalidateDb(dbName); } /** @@ -393,6 +415,7 @@ void invalidateAll(HmsClient client) { ((CachingHmsClient) client).flushAll(); } fileListingCache.invalidateAll(); + partitionViewCache.invalidateAll(); } /** @@ -422,6 +445,9 @@ void invalidatePartition(HmsClient client, String dbName, String tableName, List ((CachingHmsClient) client).invalidatePartitions(dbName, tableName, partitionValues); } fileListingCache.invalidatePartitions(dbName, tableName, partitionValues); + // PERF-06: cache A's key carries no partition-name axis (only db/table/-1/-1), so a partition-level + // change cannot be scoped finer than the whole table's single cached entry — invalidate it wholesale. + partitionViewCache.invalidateTable(dbName, tableName); } /** @@ -473,6 +499,11 @@ HiveFileListingCache fileListingCacheForTest() { return fileListingCache; } + /** Test-only: the derived listPartitions view cache (PERF-06). Never null (hive has no session=user gate). */ + ConnectorPartitionViewCache> partitionViewCacheForTest() { + return partitionViewCache; + } + private HmsClient getOrCreateClient() { if (hmsClient == null) { synchronized (this) { diff --git a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java index 07f50ca76ca4a2..00cb6b9ee8c1ec 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java @@ -55,6 +55,8 @@ import org.apache.doris.connector.api.pushdown.ConnectorLiteral; import org.apache.doris.connector.api.pushdown.FilterApplicationResult; import org.apache.doris.connector.api.scan.ConnectorPartitionValues; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; import org.apache.doris.connector.hms.HiveShowCreateTableRenderer; import org.apache.doris.connector.hms.HmsClient; import org.apache.doris.connector.hms.HmsClientException; @@ -225,6 +227,17 @@ public class HiveConnectorMetadata implements ConnectorMetadata { // default (harmless for the direct-construction tests, which inject their file sizes and never list). private final HiveFileListingCache fileListingCache; + // PERF-06 (S6): cross-query DERIVED partition-view cache A (generic ConnectorPartitionViewCache), injected by + // the owning HiveConnector; null = no cross-query derived layer (the convenience/test ctors below pass null, + // matching every existing direct-construction test). Layered ABOVE the raw per-name HMS listing served by + // CachingHmsClient: a hit skips both the derived-view BUILD (the per-name HiveWriteUtils.toPartitionValues + // parse + ConnectorPartitionInfo construction in listPartitions) and the collectPartitionNames call, keyed by + // (db, table, snapshotId=-1, schemaId=-1) — hive is snapshot-less (beginQuerySnapshot always pins -1) and its + // handle carries no schema version, so both axes are pinned "unversioned". Consumed only by listPartitions: + // getMvccPartitionView returns Optional.empty() for a real hive handle (the SPI default), so fe-core's generic + // MTMV model already falls back to listPartitions for hive — there is no second enumeration hook to wrap. + private final ConnectorPartitionViewCache> partitionViewCache; + public HiveConnectorMetadata(HmsClient hmsClient, Map properties, ConnectorContext context) { this(hmsClient, properties, context, NO_ICEBERG_SIBLING, NO_HUDI_SIBLING, NO_SIBLING_OWNER); } @@ -237,11 +250,28 @@ public HiveConnectorMetadata(HmsClient hmsClient, Map properties new HiveFileListingCache(properties)); } + /** Convenience ctor without the PERF-06 derived partition-view cache (null -> listPartitions always live). */ public HiveConnectorMetadata(HmsClient hmsClient, Map properties, ConnectorContext context, Supplier icebergSiblingSupplier, Supplier hudiSiblingSupplier, Function siblingOwnerResolver, HiveFileListingCache fileListingCache) { + this(hmsClient, properties, context, icebergSiblingSupplier, hudiSiblingSupplier, siblingOwnerResolver, + fileListingCache, null); + } + + /** + * Full ctor used by {@link HiveConnector#newMetadata}, adding the PERF-06 derived partition-view cache + * (cache A): {@code partitionViewCache} memoizes {@link #listPartitions}'s built + * {@code List}, keyed by {@code (db, table, -1, -1)}. {@code null} for the + * convenience/test ctors (no cross-query derived layer -> compute directly every call). + */ + public HiveConnectorMetadata(HmsClient hmsClient, Map properties, ConnectorContext context, + Supplier icebergSiblingSupplier, + Supplier hudiSiblingSupplier, + Function siblingOwnerResolver, + HiveFileListingCache fileListingCache, + ConnectorPartitionViewCache> partitionViewCache) { this.hmsClient = hmsClient; this.properties = properties; this.context = context; @@ -249,6 +279,7 @@ public HiveConnectorMetadata(HmsClient hmsClient, Map properties this.hudiSiblingSupplier = hudiSiblingSupplier; this.siblingOwnerResolver = siblingOwnerResolver; this.fileListingCache = fileListingCache; + this.partitionViewCache = partitionViewCache; } /** @@ -1134,6 +1165,13 @@ public List listPartitionNames(ConnectorSession session, ConnectorTableH * (until then a hive MTMV base table's per-partition freshness is UNKNOWN, harmless while hive is * dormant). {@code rowCount}/{@code sizeBytes}/{@code fileCount} are likewise UNKNOWN — hive does not * declare {@code SUPPORTS_PARTITION_STATS} (legacy SHOW PARTITIONS lists names only).

+ * + *

PERF-06 cache A: the BUILT {@code List} is memoized across queries in + * {@link #partitionViewCache}, keyed by {@code (db, table, -1, -1)} (hive is snapshot-less and carries no + * schema version — see the field javadoc) — a hit skips both {@link #collectPartitionNames} (which is itself + * already CACHED by {@code CachingHmsClient}) and the per-name value-parse/derive loop below. The cache is + * BYPASSED (compute directly, never populated) when {@code partitionViewCache} is {@code null} (the + * convenience/test ctors) or {@code filter} is present (not the pruning path, and not keyed by filter). */ @Override public List listPartitions(ConnectorSession session, @@ -1142,6 +1180,16 @@ public List listPartitions(ConnectorSession session, return siblingMetadata(session, handle).listPartitions(session, handle, filter); } HiveTableHandle hiveHandle = (HiveTableHandle) handle; + if (partitionViewCache == null || filter.isPresent()) { + return listPartitionsUncached(hiveHandle); + } + PartitionViewCacheKey key = new PartitionViewCacheKey( + hiveHandle.getDbName(), hiveHandle.getTableName(), -1L, -1L); + return partitionViewCache.get(key, () -> listPartitionsUncached(hiveHandle)); + } + + /** The derivation seam PERF-06 cache A wraps: builds the full partition-view list, uncached. */ + private List listPartitionsUncached(HiveTableHandle hiveHandle) { List partKeyNames = hiveHandle.getPartitionKeyNames(); // Query partition pruning: CACHED listing (use_meta_cache contract; legacy pruned off HiveExternalMetaCache). List partitionNames = collectPartitionNames(hiveHandle, false); diff --git a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java new file mode 100644 index 00000000000000..3b375114572190 --- /dev/null +++ b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java @@ -0,0 +1,251 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.hive; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.api.ConnectorType; +import org.apache.doris.connector.api.pushdown.ConnectorColumnRef; +import org.apache.doris.connector.api.pushdown.ConnectorComparison; +import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.api.pushdown.ConnectorLiteral; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.hms.HmsClient; +import org.apache.doris.connector.hms.HmsDatabaseInfo; +import org.apache.doris.connector.hms.HmsPartitionInfo; +import org.apache.doris.connector.hms.HmsTableInfo; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * PERF-06 (S6) tests for the cross-query DERIVED partition-view cache ("cache A", the generic + * {@link ConnectorPartitionViewCache}) wired into {@link HiveConnectorMetadata#listPartitions}. Hive does NOT + * override {@code getMvccPartitionView} for a real hive handle (it returns the SPI default + * {@code Optional.empty()} — fe-core's generic MTMV model already falls back to {@code listPartitions}), so — + * like paimon's single typed field — there is exactly ONE enumeration hook to wrap. + * + *

Uses a {@link CountingHmsClient} double (no Mockito, no docker): the derivation seam + * {@link HiveConnectorMetadata#listPartitions} wraps is {@code collectPartitionNames} -> + * {@code hmsClient.listPartitionNames}, so a cache HIT skips the whole loader (including the per-name + * {@link HiveWriteUtils#toPartitionValues} parse + {@link ConnectorPartitionInfo} construction) and the call + * count on {@code listPartitionNames} is the enumeration counter — the same seam + * {@link HiveConnectorMetadataPartitionListTest} already asserts seam-touch on. + * + *

Hive is snapshot-less (its {@code beginQuerySnapshot} always pins {@code -1}) and its + * {@link HiveTableHandle} carries no schema version, so cache A's key is always + * {@code (db, table, -1, -1)} for a given table — unlike iceberg/paimon there is no "distinct snapshot" + * dimension to re-key on; only the explicit invalidation hooks (+ TTL) can force a re-derive. + */ +public class HiveConnectorMetadataPartitionViewCacheTest { + + private static final List PART_KEYS = Arrays.asList("year", "month"); + private static final List PARTITIONS = Arrays.asList( + "year=2023/month=12", + "year=2024/month=01"); + + private static HiveConnectorMetadata metadataWithCache(CountingHmsClient client, + ConnectorPartitionViewCache> cache) { + return new HiveConnectorMetadata(client, Collections.emptyMap(), new FakeConnectorContext(), + () -> { + throw new UnsupportedOperationException(); + }, + () -> { + throw new UnsupportedOperationException(); + }, + handle -> { + throw new UnsupportedOperationException(); + }, + new HiveFileListingCache(Collections.emptyMap()), cache); + } + + private static ConnectorPartitionViewCache> partitionViewCache() { + return new ConnectorPartitionViewCache<>("hive", Collections.emptyMap()); + } + + private static HiveTableHandle handle() { + return new HiveTableHandle.Builder("db1", "t1", HiveTableType.HIVE) + .partitionKeyNames(PART_KEYS) + .build(); + } + + private static List names(List infos) { + return infos.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList()); + } + + @Test + public void listPartitionsCachesDerivedListAcrossQueries() { + // WHY: cache A must memoize the BUILT List keyed by (db, table, -1, -1), so a + // repeated query on the same table skips the derived rebuild AND the hmsClient.listPartitionNames + // round-trip. MUTATION: not consulting the cache (compute directly every call) -> the seam runs twice + // -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + List first = md.listPartitions(null, h, Optional.empty()); + List second = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertEquals(PARTITIONS, names(first)); + Assertions.assertEquals(names(first), names(second), "the cached list is returned verbatim"); + Assertions.assertEquals(1, client.listPartitionNamesCalls, + "a cache hit must not re-enumerate (listPartitionNames once)"); + } + + @Test + public void listPartitionsInvalidateTableForcesReEnumeration() { + // WHY: REFRESH TABLE (HiveConnector.invalidateTable -> cache.invalidateTable) must drop the cached list + // so the next query re-enumerates live. MUTATION: invalidateTable not wired -> the second call hits the + // stale entry -> listPartitionNamesCalls stays 1 -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateTable("db1", "t1"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, client.listPartitionNamesCalls, "invalidateTable must force a re-enumeration"); + } + + @Test + public void listPartitionsInvalidateAllForcesReEnumeration() { + // WHY: REFRESH CATALOG (HiveConnector.invalidateAll -> cache.invalidateAll) must drop the cached list. + // MUTATION: invalidateAll not wired -> the second call hits -> listPartitionNamesCalls stays 1 -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateAll(); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, client.listPartitionNamesCalls, "invalidateAll must force a re-enumeration"); + } + + @Test + public void listPartitionsWithFilterBypassesCache() { + // WHY: a present filter must BYPASS the cache and compute directly -- and must NOT populate it either, so + // a later empty-filter (pruning) call still misses. Hive already ignores the filter value entirely + // (returns the full set regardless), but the cache-A key carries no filter dimension, so caching a + // filtered call's result would be indistinguishable from caching an unfiltered one; bypassing keeps the + // two paths independent, mirroring the iceberg/paimon pattern. MUTATION: caching regardless of filter -> + // the second filtered call hits (count stays 1) -> red; or a bypassed call populating the cache -> the + // following empty-filter call hits instead of missing -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + ConnectorExpression filter = new ConnectorComparison(ConnectorComparison.Operator.EQ, + new ConnectorColumnRef("year", ConnectorType.of("STRING")), + new ConnectorLiteral(ConnectorType.of("STRING"), "2024")); + md.listPartitions(null, h, Optional.of(filter)); + md.listPartitions(null, h, Optional.of(filter)); + Assertions.assertEquals(2, client.listPartitionNamesCalls, + "a present filter must bypass the cache (compute every call)"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(3, client.listPartitionNamesCalls, + "the empty-filter call must miss (filtered calls never populate)"); + } + + @Test + public void listPartitionsNullCacheEnumeratesEveryCall() { + // The convenience/test-ctor analogue: a null cache means compute directly every call -> the seam runs on + // every query (no cross-query sharing). MUTATION: defaulting to a shared cache when null was intended -> + // listPartitionNamesCalls stays 1 -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + HiveConnectorMetadata md = metadataWithCache(client, null); + HiveTableHandle h = handle(); + + md.listPartitions(null, h, Optional.empty()); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, client.listPartitionNamesCalls, "a null (disabled) cache must re-enumerate every call"); + } + + /** + * Minimal {@link HmsClient} double: {@code listPartitionNames} returns a fixed list and counts calls; + * {@code getPartitions} fails loud (the per-partition round-trip {@link HiveConnectorMetadata#listPartitions} + * must never make — mirrors {@link HiveConnectorMetadataPartitionListTest}'s FakeHmsClient). + */ + private static final class CountingHmsClient implements HmsClient { + private final List partitionNames; + int listPartitionNamesCalls; + + CountingHmsClient(List partitionNames) { + this.partitionNames = partitionNames; + } + + @Override + public List listPartitionNames(String dbName, String tableName, int maxParts) { + listPartitionNamesCalls++; + return partitionNames; + } + + @Override + public List getPartitions(String dbName, String tableName, List partNames) { + throw new AssertionError("get_partitions_by_names must not be called by partition listing"); + } + + @Override + public List listDatabases() { + throw new UnsupportedOperationException(); + } + + @Override + public HmsDatabaseInfo getDatabase(String dbName) { + throw new UnsupportedOperationException(); + } + + @Override + public List listTables(String dbName) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean tableExists(String dbName, String tableName) { + throw new UnsupportedOperationException(); + } + + @Override + public HmsTableInfo getTable(String dbName, String tableName) { + throw new UnsupportedOperationException(); + } + + @Override + public Map getDefaultColumnValues(String dbName, String tableName) { + throw new UnsupportedOperationException(); + } + + @Override + public HmsPartitionInfo getPartition(String dbName, String tableName, List values) { + throw new UnsupportedOperationException(); + } + + @Override + public void close() { + } + } +} diff --git a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java new file mode 100644 index 00000000000000..57c765f9482e90 --- /dev/null +++ b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java @@ -0,0 +1,144 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.hive; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; + +/** + * PERF-06 (S6) tests for {@link HiveConnector}'s partition-view cache A wiring: that the cache is ALWAYS built + * (no session=user gate — hive has no per-query-identity cache-disabling convention; its caches, like + * {@link HiveFileListingCache}, are already built unconditionally per-catalog) and that the REFRESH hooks + + * {@code invalidatePartition} route to it. + */ +public class HiveConnectorPartitionViewCacheTest { + + private static Map props() { + Map m = new HashMap<>(); + m.put("hive.metastore.uris", "thrift://host:9083"); + return m; + } + + @Test + public void partitionViewCacheAlwaysBuilt() { + // Unlike iceberg, hive has NO session=user / per-user credential-isolation cache-disabling convention (a + // hive catalog authenticates at catalog-creation time, not per-query session identity), so the connector + // must construct cache A unconditionally. MUTATION: a stray session-like gate leaving the field null on + // some property combination -> red. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + Assertions.assertNotNull(connector.partitionViewCacheForTest(), + "a fresh hive connector must always build the partition-view cache"); + } + + @Test + public void refreshHooksInvalidatePartitionViewCache() { + // The REFRESH hooks must clear cache A too (else external DDL/writes stay invisible beyond the TTL): + // REFRESH TABLE drops that table's entry, REFRESH DATABASE that db's, REFRESH CATALOG everything. + // Asserted via a counting loader (the framework's size() is package-private): after invalidation the + // loader must run again. MUTATION: an invalidate* hook not routed to the view cache -> the entry + // survives -> loader not re-run -> a loads assert below red. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + ConnectorPartitionViewCache> cache = connector.partitionViewCacheForTest(); + Assertions.assertNotNull(cache); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", -1L, -1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", -1L, -1L); + PartitionViewCacheKey db2t1 = new PartitionViewCacheKey("db2", "t1", -1L, -1L); + + // REFRESH TABLE db1.t1 -> only db1.t1 re-loads. Uses the public no-client hook (mirrors a never-scanned + // catalog's REFRESH — hmsClient is null here). + cache.get(db1t1, loader); + cache.get(db1t1, loader); + Assertions.assertEquals(1, loads[0], "second get is a hit"); + connector.invalidateTable("db1", "t1"); + cache.get(db1t1, loader); + Assertions.assertEquals(2, loads[0], "REFRESH TABLE forces a reload of db1.t1"); + + // REFRESH DATABASE db1 -> db1.t2 re-loads; db2.t1 unaffected. + cache.get(db1t2, loader); // loads=3 (miss) + cache.get(db2t1, loader); // loads=4 (miss) + cache.get(db1t2, loader); // hit + cache.get(db2t1, loader); // hit + Assertions.assertEquals(4, loads[0]); + connector.invalidateDb("db1"); + cache.get(db2t1, loader); // db2 untouched -> hit + Assertions.assertEquals(4, loads[0], "REFRESH DATABASE db1 must NOT drop db2's entries"); + cache.get(db1t2, loader); // db1.t2 dropped -> miss + Assertions.assertEquals(5, loads[0], "REFRESH DATABASE db1 drops db1's entries"); + + // REFRESH CATALOG -> everything re-loads. + connector.invalidateAll(); + cache.get(db2t1, loader); + Assertions.assertEquals(6, loads[0], "REFRESH CATALOG drops everything"); + } + + @Test + public void invalidatePartitionDropsTheWholeTablesCachedView() { + // Cache A's key carries no partition-name axis (only db/table/-1/-1), so a partition add/drop/alter + // (HiveConnector.invalidatePartition) cannot be scoped finer than the table's single cached entry -- + // it must invalidate that whole entry. MUTATION: invalidatePartition not routed to the view cache -> the + // stale entry survives -> the reload assert below red. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + ConnectorPartitionViewCache> cache = connector.partitionViewCacheForTest(); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", -1L, -1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", -1L, -1L); + + cache.get(db1t1, loader); + cache.get(db1t2, loader); + Assertions.assertEquals(2, loads[0]); + + connector.invalidatePartition("db1", "t1", Collections.singletonList("dt=2024-01-01")); + + cache.get(db1t1, loader); + Assertions.assertEquals(3, loads[0], "invalidatePartition must drop the whole cached view of that table"); + cache.get(db1t2, loader); + Assertions.assertEquals(3, loads[0], "invalidatePartition must NOT drop another table's cached view"); + } + + @Test + public void publicHooksAreNoThrowWithoutBuildingAClient() { + // A fresh connector never built its metastore client (hmsClient == null). The public REFRESH hooks must + // not force-build one just to route to the view cache, and must not throw. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + Assertions.assertDoesNotThrow(() -> connector.invalidateTable("db", "t")); + Assertions.assertDoesNotThrow(() -> connector.invalidateDb("db")); + Assertions.assertDoesNotThrow(() -> connector.invalidateAll()); + Assertions.assertDoesNotThrow(() -> { + connector.invalidatePartition("db", "t", Collections.singletonList("dt=2024-01-01")); + }); + } +} diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java index 47fb3dd5abd2ae..95677a9dd8c023 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java @@ -20,14 +20,17 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorCapability; import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorPartitionInfo; import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorTestResult; import org.apache.doris.connector.api.ConnectorValidationContext; import org.apache.doris.connector.api.DorisConnectorException; import org.apache.doris.connector.api.handle.ConnectorTableHandle; +import org.apache.doris.connector.api.mvcc.ConnectorMvccPartitionView; import org.apache.doris.connector.api.procedure.ConnectorProcedureOps; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; import org.apache.doris.connector.api.write.ConnectorWritePlanProvider; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; import org.apache.doris.connector.metastore.HmsMetaStoreProperties; import org.apache.doris.connector.metastore.spi.JdbcDriverSupport; import org.apache.doris.connector.metastore.spi.MetaStoreProviders; @@ -76,6 +79,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -181,6 +185,19 @@ public class IcebergConnector implements Connector { // the comment path, and session=user must stay live because the loadTable itself carries per-user // authorization a shared cache would bypass. null for every other flavor. private final IcebergCommentCache commentCache; // authz-cache-session-user-disabled + // PERF-06: cross-query DERIVED partition-view cache ("cache A", the generic ConnectorPartitionViewCache from + // fe-connector-cache), layered ABOVE the raw partitionCache (PERF-02): it memoizes the BUILT derived view + // (transform-to-range math + overlap merge for the MTMV view; the value-map construction for listPartitions) + // keyed by (db, table, snapshotId, schemaId), so a repeated query on a partitioned table skips the derived + // rebuild, not just the remote scan. Two typed fields because the two SPI hooks return structurally different + // derived types and neither derives from the other; the shared raw PARTITIONS scan underneath is already + // deduped by partitionCache, so two derived caches never double-scan remotely. Authorization-sensitive + // projection like partitionCache/formatCache: disabled (null) under iceberg.rest.session=user so a shared + // (no user dimension) hit cannot disclose one user's partition view; kept for every other flavor. + private final ConnectorPartitionViewCache // authz-cache-session-user-disabled + mvccPartitionViewCache; + private final ConnectorPartitionViewCache> // authz-cache-session-user-disabled + listPartitionsViewCache; // Manifest content cache — pure metadata, default-off (meta.cache.iceberg.manifest.enable), and consumed // ONLY after a per-user resolveTable(ForRead). authz-cache-exempt (no read path without a per-user load). private final IcebergManifestCache manifestCache = new IcebergManifestCache(); @@ -254,6 +271,17 @@ public IcebergConnector(Map properties, ConnectorContext context ? new IcebergCommentCache( resolveTableCacheTtlSecond(this.properties), DEFAULT_TABLE_CACHE_CAPACITY) : null; + // PERF-06: derived partition-view cache A (generic ConnectorPartitionViewCache). Same + // authorization-sensitive treatment as partitionCache -- disabled (null) under iceberg.rest.session=user so + // a shared (no user dimension) hit cannot disclose one user's partition view; kept otherwise. Reads its + // own meta.cache.iceberg.partition_view.(enable|ttl-second|capacity) from the catalog properties via the + // framework's CacheSpec (default ON / 24h / 1000). Two typed instances (MVCC view + partition-info list). + this.mvccPartitionViewCache = isUserSessionEnabled() + ? null + : new ConnectorPartitionViewCache<>("iceberg", this.properties); + this.listPartitionsViewCache = isUserSessionEnabled() + ? null + : new ConnectorPartitionViewCache<>("iceberg", this.properties); } /** @@ -278,7 +306,8 @@ static long resolveTableCacheTtlSecond(Map properties) { @Override public ConnectorMetadata getMetadata(ConnectorSession session) { return new IcebergConnectorMetadata(newCatalogBackedOps(session), properties, context, - latestSnapshotCache, tableCache, partitionCache, commentCache); + latestSnapshotCache, tableCache, partitionCache, commentCache, + mvccPartitionViewCache, listPartitionsViewCache); } /** @@ -603,6 +632,12 @@ public void invalidateTable(String dbName, String tableName) { if (commentCache != null) { commentCache.invalidate(TableIdentifier.of(dbName, tableName)); } + if (mvccPartitionViewCache != null) { + mvccPartitionViewCache.invalidateTable(dbName, tableName); + } + if (listPartitionsViewCache != null) { + listPartitionsViewCache.invalidateTable(dbName, tableName); + } } /** @@ -633,6 +668,12 @@ public void invalidateDb(String dbName) { if (commentCache != null) { commentCache.invalidateDb(dbName); } + if (mvccPartitionViewCache != null) { + mvccPartitionViewCache.invalidateDb(dbName); + } + if (listPartitionsViewCache != null) { + listPartitionsViewCache.invalidateDb(dbName); + } } /** @@ -659,6 +700,12 @@ public void invalidateAll() { if (commentCache != null) { commentCache.invalidateAll(); } + if (mvccPartitionViewCache != null) { + mvccPartitionViewCache.invalidateAll(); + } + if (listPartitionsViewCache != null) { + listPartitionsViewCache.invalidateAll(); + } manifestCache.invalidateAll(); } @@ -713,6 +760,16 @@ IcebergCommentCache commentCacheForTest() { return commentCache; } + /** Test-only: the derived MVCC partition-view cache (PERF-06), or {@code null} for a session=user catalog. */ + ConnectorPartitionViewCache mvccPartitionViewCacheForTest() { + return mvccPartitionViewCache; + } + + /** Test-only: the derived listPartitions view cache (PERF-06), or {@code null} for a session=user catalog. */ + ConnectorPartitionViewCache> listPartitionsViewCacheForTest() { + return listPartitionsViewCache; + } + @Override public ConnectorScanPlanProvider getScanPlanProvider() { // Mirrors PaimonConnector.getScanPlanProvider: build a fresh provider per call over the lazily-built diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java index 70f5d0a146e056..99a54b500ee866 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java @@ -41,6 +41,8 @@ import org.apache.doris.connector.api.mvcc.ConnectorMvccSnapshot; import org.apache.doris.connector.api.mvcc.ConnectorTimeTravelSpec; import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.thrift.THiveTable; import org.apache.doris.thrift.TIcebergTable; @@ -155,6 +157,14 @@ public class IcebergConnectorMetadata implements ConnectorMetadata { // connector is a REST vended-credentials, non-session catalog (see IcebergConnector); the convenience ctors // used by direct-construction tests pass null. Consumed only by getTableComment. private final IcebergCommentCache commentCache; + // PERF-06: cross-query DERIVED partition-view cache A (generic ConnectorPartitionViewCache), injected by the + // owning IcebergConnector; null = no cross-query derived layer (the convenience ctors used by + // direct-construction tests pass null; a session=user catalog also passes null). Layered ABOVE partitionCache + // (raw rows): a hit skips the derived-view BUILD, keyed by (db, table, snapshotId, schemaId). Two typed fields + // because getMvccPartitionView and listPartitions return different derived types. Consumed by + // getMvccPartitionView / listPartitions respectively. + private final ConnectorPartitionViewCache mvccPartitionViewCache; + private final ConnectorPartitionViewCache> listPartitionsViewCache; public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map properties, ConnectorContext context) { @@ -180,11 +190,28 @@ public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map properties, ConnectorContext context, IcebergLatestSnapshotCache latestSnapshotCache, IcebergTableCache tableCache, IcebergPartitionCache partitionCache, IcebergCommentCache commentCache) { + this(catalogOps, properties, context, latestSnapshotCache, tableCache, partitionCache, commentCache, + null, null); + } + + /** + * Full ctor used by {@link IcebergConnector#getMetadata}, adding the PERF-06 derived partition-view caches + * (cache A): {@code mvccPartitionViewCache} memoizes {@link #getMvccPartitionView}'s built RANGE view and + * {@code listPartitionsViewCache} memoizes {@link #listPartitions}'s built partition-info list, each keyed by + * {@code (db, table, snapshotId, schemaId)}. Both {@code null} for a session=user catalog / the convenience + * ctors (no cross-query derived layer -> compute directly every call). + */ + public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map properties, + ConnectorContext context, IcebergLatestSnapshotCache latestSnapshotCache, + IcebergTableCache tableCache, IcebergPartitionCache partitionCache, + IcebergCommentCache commentCache, + ConnectorPartitionViewCache mvccPartitionViewCache, + ConnectorPartitionViewCache> listPartitionsViewCache) { this.catalogOps = catalogOps; this.properties = properties; this.context = context; @@ -192,6 +219,8 @@ public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map getMvccPartitionView( ConnectorSession session, ConnectorTableHandle handle) { IcebergTableHandle iceHandle = (IcebergTableHandle) handle; try { + // PERF-06 cache A: memoize the BUILT derived view keyed by (db, table, snapshotId, schemaId) -- pure + // function of the pinned MVCC coordinate (a new snapshot/schema yields a new key, never a stale hit). + // The lookup sits INSIDE executeAuthenticated so a miss runs the loader (resolveTableForRead + the + // remote PARTITIONS build) under the FE-injected auth scope; a hit returns without any remote call. A + // null cache (session=user / no-cache catalog) computes directly every call. -1 (empty table / unpinned) + // enumerates the current snapshot and caches a trivially-empty view (harmless; REFRESH re-pins). return context.executeAuthenticated(() -> { - Table table = resolveTableForRead(session, iceHandle); - return Optional.of(IcebergPartitionUtils.buildMvccPartitionView(table, iceHandle.getSnapshotId(), - TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache)); + if (mvccPartitionViewCache == null) { + return Optional.of(buildMvccPartitionViewUncached(session, iceHandle)); + } + PartitionViewCacheKey key = new PartitionViewCacheKey(iceHandle.getDbName(), + iceHandle.getTableName(), iceHandle.getSnapshotId(), iceHandle.getSchemaId()); + return Optional.of(mvccPartitionViewCache.get(key, + () -> buildMvccPartitionViewUncached(session, iceHandle))); }); } catch (Exception e) { throw new RuntimeException("Failed to build iceberg MVCC partition view, error message is:" @@ -1531,6 +1570,19 @@ public Optional getMvccPartitionView( } } + /** + * Builds the derived MVCC partition view live (no cache-A layer): the same resolveTableForRead + remote + * PARTITIONS build the pre-cache code ran. Extracted so it can serve as cache A's per-miss loader. MUST run + * inside {@code context.executeAuthenticated} (the caller wraps it). A {@code NoSuchTableException} propagates + * (fail-loud parity) and, running through the framework loader, is never cached. + */ + private ConnectorMvccPartitionView buildMvccPartitionViewUncached( + ConnectorSession session, IcebergTableHandle iceHandle) { + Table table = resolveTableForRead(session, iceHandle); + return IcebergPartitionUtils.buildMvccPartitionView(table, iceHandle.getSnapshotId(), + TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache); + } + /** * The physical iceberg partition display names ({@code "f1=v1/f2=v2"}) for SHOW PARTITIONS (single-column * form — iceberg does not declare {@code SUPPORTS_PARTITION_STATS}). Post-cutover this restores real rows @@ -1574,17 +1626,18 @@ public List listPartitions(ConnectorSession session, ConnectorTableHandle handle, Optional filter) { IcebergTableHandle iceHandle = (IcebergTableHandle) handle; try { + // PERF-06 cache A: memoize the BUILT partition-info list keyed by (db, table, snapshotId, schemaId). + // The lookup sits INSIDE executeAuthenticated (a miss runs the remote build under the auth scope; a hit + // returns without a remote call). BYPASS the cache when the filter is present -- that is not the + // pruning path (which always passes Optional.empty()) and is not keyed by (snapshot, schema) alone -- or + // when the cache is null (session=user / no-cache catalog): compute directly every call. return context.executeAuthenticated(() -> { - Table table; - try { - table = resolveTableForRead(session, iceHandle); - } catch (NoSuchTableException e) { - LOG.warn("Iceberg table not found while listing partitions: {}.{}", - iceHandle.getDbName(), iceHandle.getTableName(), e); - return Collections.emptyList(); + if (listPartitionsViewCache == null || filter.isPresent()) { + return listPartitionsUncached(session, iceHandle); } - return IcebergPartitionUtils.listPartitions(table, - TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache); + PartitionViewCacheKey key = new PartitionViewCacheKey(iceHandle.getDbName(), + iceHandle.getTableName(), iceHandle.getSnapshotId(), iceHandle.getSchemaId()); + return listPartitionsViewCache.get(key, () -> listPartitionsUncached(session, iceHandle)); }); } catch (Exception e) { throw new RuntimeException("Failed to list iceberg partitions, error message is:" @@ -1592,6 +1645,26 @@ public List listPartitions(ConnectorSession session, } } + /** + * Builds the derived partition-info list live (no cache-A layer): the same resolveTableForRead + remote + * PARTITIONS build the pre-cache code ran, including the concurrent-drop degrade (a {@code NoSuchTableException} + * yields an empty list). Extracted so it can serve as cache A's per-miss loader. MUST run inside + * {@code context.executeAuthenticated} (the caller wraps it). + */ + private List listPartitionsUncached( + ConnectorSession session, IcebergTableHandle iceHandle) { + Table table; + try { + table = resolveTableForRead(session, iceHandle); + } catch (NoSuchTableException e) { + LOG.warn("Iceberg table not found while listing partitions: {}.{}", + iceHandle.getDbName(), iceHandle.getTableName(), e); + return Collections.emptyList(); + } + return IcebergPartitionUtils.listPartitions(table, + TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache); + } + // ========== E5: MVCC snapshots / time travel ========== /** diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java index b850f84c63fdc4..14502c4ff077e3 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java @@ -1050,7 +1050,13 @@ private List readExistingFileScopedDeletes( || !ContentFileUtil.isFileScoped(deleteFile)) { continue; } - String referenced = deleteFile.referencedDataFile(); + // Use the bounds-aware ContentFileUtil accessor, consistent with the isFileScoped() gate + // above: a Spark-written file-scoped parquet/orc position delete carries its referenced + // data file in the file_path column bounds, NOT in the manifest referenced_data_file field + // (id 143) — only a v3 puffin DV sets that field. The raw DeleteFile.referencedDataFile() + // therefore returns null for such deletes, so they would pass isFileScoped() yet be dropped + // here, leaving the superseded parquet delete live after a v3 DELETE. + String referenced = ContentFileUtil.referencedDataFileLocation(deleteFile); if (referenced == null || !touchedDataFilePaths.contains(referenced)) { continue; } diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java index 0a22ffff2fb6e4..5d5b5285f07d43 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java @@ -17,6 +17,10 @@ package org.apache.doris.connector.iceberg; +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; + import org.apache.iceberg.DataFiles; import org.apache.iceberg.ManifestFile; import org.apache.iceberg.PartitionSpec; @@ -31,8 +35,10 @@ import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.OptionalLong; +import java.util.function.Supplier; /** * Tests IcebergConnector's T08 cache knobs: the latest-snapshot cache TTL resolution @@ -451,4 +457,97 @@ public void refreshHooksInvalidateCommentCache() { connector.invalidateAll(); Assertions.assertEquals(0, cache.size(), "REFRESH CATALOG drops everything"); } + + // ============ PERF-06: derived partition-view cache A (session=user gated) + invalidation ============ + + @Test + public void partitionViewCacheBuiltUnlessSessionUser() { + // Cache A (the derived partition-view cache) stores pure metadata (a partition view carries no + // FileIO/credential), so like the partition cache it stays built for a REST vended-credentials catalog. But + // under iceberg.rest.session=user it is an AUTHORIZATION-sensitive projection -- a shared (no user + // dimension) hit would disclose one user's partition view -- so BOTH typed instances are disabled (null) + // there. MUTATION: dropping the session=user gate -> non-null for session -> red; gating on the vended flag + // -> null for vended -> red. + IcebergConnector plain = new IcebergConnector(Collections.emptyMap(), new RecordingConnectorContext()); + Assertions.assertNotNull(plain.mvccPartitionViewCacheForTest(), "a plain catalog builds the MVCC view cache"); + Assertions.assertNotNull(plain.listPartitionsViewCacheForTest(), + "a plain catalog builds the listPartitions view cache"); + Map vended = new HashMap<>(); + vended.put(IcebergConnectorProperties.ICEBERG_CATALOG_TYPE, IcebergConnectorProperties.TYPE_REST); + vended.put(IcebergConnectorProperties.REST_VENDED_CREDENTIALS_ENABLED, "true"); + IcebergConnector vendedConn = new IcebergConnector(vended, new RecordingConnectorContext()); + Assertions.assertNotNull(vendedConn.mvccPartitionViewCacheForTest(), + "a vended-credentials catalog still builds the MVCC view cache (metadata carries no credentials)"); + Assertions.assertNotNull(vendedConn.listPartitionsViewCacheForTest(), + "a vended-credentials catalog still builds the listPartitions view cache"); + Map session = new HashMap<>(); + session.put(IcebergConnectorProperties.ICEBERG_CATALOG_TYPE, IcebergConnectorProperties.TYPE_REST); + session.put(IcebergConnectorProperties.REST_SESSION, IcebergConnectorProperties.SESSION_USER); + IcebergConnector sessionConn = new IcebergConnector(session, new RecordingConnectorContext()); + Assertions.assertNull(sessionConn.mvccPartitionViewCacheForTest(), + "a session=user catalog must NOT build the MVCC view cache (per-user authz must not be bypassed)"); + Assertions.assertNull(sessionConn.listPartitionsViewCacheForTest(), + "a session=user catalog must NOT build the listPartitions view cache"); + } + + @Test + public void refreshHooksInvalidatePartitionViewCache() { + // The REFRESH hooks must clear cache A too (else external DDL/writes stay invisible beyond the pin): REFRESH + // TABLE drops that table's snapshot entries, REFRESH DATABASE that db's, REFRESH CATALOG everything. Asserted + // via a counting loader (the framework's size() is package-private): after invalidation the loader must run + // again. MUTATION: an invalidate* hook not routed to the view cache -> the entry survives -> loader not + // re-run -> a loads assert below red. + IcebergConnector connector = + new IcebergConnector(Collections.emptyMap(), new RecordingConnectorContext()); + ConnectorPartitionViewCache> cache = connector.listPartitionsViewCacheForTest(); + Assertions.assertNotNull(cache); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", 1L, 1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", 1L, 1L); + PartitionViewCacheKey db2t1 = new PartitionViewCacheKey("db2", "t1", 1L, 1L); + + // REFRESH TABLE db1.t1 -> only db1.t1 re-loads. + cache.get(db1t1, loader); + cache.get(db1t1, loader); + Assertions.assertEquals(1, loads[0], "second get is a hit"); + connector.invalidateTable("db1", "t1"); + cache.get(db1t1, loader); + Assertions.assertEquals(2, loads[0], "REFRESH TABLE forces a reload of db1.t1"); + + // REFRESH DATABASE db1 -> db1.t2 re-loads; db2.t1 unaffected. + cache.get(db1t2, loader); // loads=3 (miss) + cache.get(db2t1, loader); // loads=4 (miss) + cache.get(db1t2, loader); // hit + cache.get(db2t1, loader); // hit + Assertions.assertEquals(4, loads[0]); + connector.invalidateDb("db1"); + cache.get(db2t1, loader); // db2 untouched -> hit + Assertions.assertEquals(4, loads[0], "REFRESH DATABASE db1 must NOT drop db2's entries"); + cache.get(db1t2, loader); // db1.t2 dropped -> miss + Assertions.assertEquals(5, loads[0], "REFRESH DATABASE db1 drops db1's entries"); + + // REFRESH CATALOG -> everything re-loads. + connector.invalidateAll(); + cache.get(db2t1, loader); + Assertions.assertEquals(6, loads[0], "REFRESH CATALOG drops everything"); + } + + @Test + public void invalidateHooksNoThrowForSessionUserPartitionViewCaches() { + // Under session=user cache A's two instances are null; the invalidate hooks must null-guard them (no NPE). + // MUTATION: an unguarded view-cache invalidate on a nulled field -> NPE -> red. + Map session = new HashMap<>(); + session.put(IcebergConnectorProperties.ICEBERG_CATALOG_TYPE, IcebergConnectorProperties.TYPE_REST); + session.put(IcebergConnectorProperties.REST_SESSION, IcebergConnectorProperties.SESSION_USER); + IcebergConnector connector = new IcebergConnector(session, new RecordingConnectorContext()); + Assertions.assertNull(connector.mvccPartitionViewCacheForTest()); + Assertions.assertNull(connector.listPartitionsViewCacheForTest()); + Assertions.assertDoesNotThrow(() -> connector.invalidateTable("db1", "t1")); + Assertions.assertDoesNotThrow(() -> connector.invalidateDb("db1")); + Assertions.assertDoesNotThrow(connector::invalidateAll); + } } diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java new file mode 100644 index 00000000000000..0ca659a7bd24f1 --- /dev/null +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java @@ -0,0 +1,260 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.iceberg; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.api.mvcc.ConnectorMvccPartition; +import org.apache.doris.connector.api.mvcc.ConnectorMvccPartitionView; +import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; + +import org.apache.iceberg.DataFiles; +import org.apache.iceberg.FileFormat; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * PERF-06 tests for the cross-query DERIVED partition-view cache ("cache A", the generic + * {@link ConnectorPartitionViewCache}) wired into {@link IcebergConnectorMetadata#getMvccPartitionView} / + * {@link IcebergConnectorMetadata#listPartitions}. Uses the real {@link InMemoryCatalog} + + * {@link RecordingIcebergCatalogOps} harness (no Mockito, no docker): the cache sits ABOVE the per-query build, + * whose first step is {@code resolveTableForRead -> catalogOps.loadTable} (logged as {@code loadTable:db1.t1}), so + * a cache HIT skips the whole loader and the {@code loadTable} count is the enumeration counter — the SAME proxy + * the sibling cache tests use ({@code IcebergConnectorMetadataMvccTest.beginQuerySnapshot*Cache*}). The raw + * partition cache (PERF-02) is passed null throughout, so the {@code loadTable} count isolates cache A's effect. + * The partition math/merge parity itself is covered by {@link IcebergPartitionUtilsTest}. + */ +public class IcebergConnectorMetadataPartitionViewCacheTest { + + private static final Schema PARTITIONED_SCHEMA = new Schema( + Types.NestedField.required(1, "id", Types.IntegerType.get()), + Types.NestedField.optional(2, "ts", Types.TimestampType.withoutZone())); + + /** A real db1.t1 partitioned by day(ts) carrying TWO snapshots (day=100, then +day=101). */ + private static final class TwoSnap { + Table table; + long s1; + long s2; + long schemaId; + } + + private static TwoSnap twoSnapshotTable() { + InMemoryCatalog catalog = new InMemoryCatalog(); + catalog.initialize("test", Collections.emptyMap()); + catalog.createNamespace(Namespace.of("db1")); + PartitionSpec spec = PartitionSpec.builderFor(PARTITIONED_SCHEMA).day("ts").build(); + Table table = catalog.createTable(TableIdentifier.of("db1", "t1"), PARTITIONED_SCHEMA, spec); + table.newAppend().appendFile(dayFile(spec, "s3://b/db1/t1/f1.parquet", "ts_day=1970-04-11")).commit(); + TwoSnap f = new TwoSnap(); + f.s1 = table.currentSnapshot().snapshotId(); + table.newAppend().appendFile(dayFile(spec, "s3://b/db1/t1/f2.parquet", "ts_day=1970-04-12")).commit(); + f.s2 = table.currentSnapshot().snapshotId(); + f.schemaId = table.schema().schemaId(); + f.table = catalog.loadTable(TableIdentifier.of("db1", "t1")); + Assertions.assertNotEquals(f.s1, f.s2, "two appends must create two distinct snapshots"); + return f; + } + + private static org.apache.iceberg.DataFile dayFile(PartitionSpec spec, String path, String partitionPath) { + return DataFiles.builder(spec) + .withPath(path).withFileSizeInBytes(100).withRecordCount(1) + .withPartitionPath(partitionPath).withFormat(FileFormat.PARQUET).build(); + } + + private static IcebergConnectorMetadata metadataWithMvccCache(RecordingIcebergCatalogOps ops, + ConnectorPartitionViewCache cache) { + // 9-arg ctor: disabled latest-snapshot cache + null table/partition/comment caches so the loadTable count + // reflects cache A alone; only the mvcc view cache under test is injected. + return new IcebergConnectorMetadata(ops, Collections.emptyMap(), new RecordingConnectorContext(), + new IcebergLatestSnapshotCache(0L, 1), null, null, null, cache, null); + } + + private static IcebergConnectorMetadata metadataWithListCache(RecordingIcebergCatalogOps ops, + ConnectorPartitionViewCache> cache) { + return new IcebergConnectorMetadata(ops, Collections.emptyMap(), new RecordingConnectorContext(), + new IcebergLatestSnapshotCache(0L, 1), null, null, null, null, cache); + } + + private static IcebergTableHandle handle() { + return new IcebergTableHandle("db1", "t1"); + } + + private static long loadCount(RecordingIcebergCatalogOps ops) { + return ops.log.stream().filter(s -> s.equals("loadTable:db1.t1")).count(); + } + + private static ConnectorPartitionViewCache mvccCache() { + return new ConnectorPartitionViewCache<>("iceberg", Collections.emptyMap()); + } + + private static ConnectorPartitionViewCache> listCache() { + return new ConnectorPartitionViewCache<>("iceberg", Collections.emptyMap()); + } + + private static List mvccNames(Optional view) { + return view.get().getPartitions().stream() + .map(ConnectorMvccPartition::getName).collect(Collectors.toList()); + } + + // --------------------------------------------------------------------- + // getMvccPartitionView + // --------------------------------------------------------------------- + + @Test + public void getMvccPartitionViewCachesDerivedViewAcrossQueries() { + // WHY: cache A must memoize the BUILT MVCC view keyed by (db, table, snapshotId, schemaId), so a repeated + // query on the same pin skips the derived rebuild AND the underlying loadTable/scan. MUTATION: not + // consulting the cache (compute directly every call) -> loadTable runs twice -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache cache = mvccCache(); + IcebergConnectorMetadata md = metadataWithMvccCache(ops, cache); + + Optional first = md.getMvccPartitionView(null, handle()); + Optional second = md.getMvccPartitionView(null, handle()); + + Assertions.assertEquals(java.util.Arrays.asList("ts_day=100", "ts_day=101"), mvccNames(first)); + Assertions.assertEquals(mvccNames(first), mvccNames(second), "the cached view is returned verbatim"); + Assertions.assertEquals(1, loadCount(ops), "a cache hit must not re-enumerate (loadTable once)"); + } + + @Test + public void getMvccPartitionViewDifferentSnapshotReEnumerates() { + // WHY: pinning a different snapshot id yields a different cache key (time-travel must not serve another + // snapshot's view), so each pin re-enumerates. MUTATION: keying on (db,table) only -> the S1 view would be + // served for the S2 pin -> names/loadCount below red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + IcebergConnectorMetadata md = metadataWithMvccCache(ops, mvccCache()); + + IcebergTableHandle atS1 = handle().withSnapshot(f.s1, null, f.schemaId); + IcebergTableHandle atS2 = handle().withSnapshot(f.s2, null, f.schemaId); + List namesS1 = mvccNames(md.getMvccPartitionView(null, atS1)); + List namesS2 = mvccNames(md.getMvccPartitionView(null, atS2)); + + Assertions.assertEquals(Collections.singletonList("ts_day=100"), namesS1); + Assertions.assertEquals(java.util.Arrays.asList("ts_day=100", "ts_day=101"), namesS2); + Assertions.assertEquals(2, loadCount(ops), "distinct snapshot keys must each enumerate"); + } + + @Test + public void getMvccPartitionViewInvalidateTableForcesReEnumeration() { + // WHY: REFRESH TABLE (Connector.invalidateTable -> cache.invalidateTable) must drop the cached view so the + // next query re-enumerates live. MUTATION: invalidateTable not wired -> the second call hits the stale + // entry -> loadCount stays 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache cache = mvccCache(); + IcebergConnectorMetadata md = metadataWithMvccCache(ops, cache); + + md.getMvccPartitionView(null, handle()); + cache.invalidateTable("db1", "t1"); + md.getMvccPartitionView(null, handle()); + Assertions.assertEquals(2, loadCount(ops), "invalidateTable must force a re-enumeration"); + } + + @Test + public void getMvccPartitionViewNullCacheEnumeratesEveryCall() { + // The session=user analogue at the metadata level: a null cache (IcebergConnector passes null under + // iceberg.rest.session=user) means compute directly every call -> loadTable on every query (no cross-query + // sharing). MUTATION: defaulting to a shared cache when null was intended -> loadCount 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + IcebergConnectorMetadata md = metadataWithMvccCache(ops, null); + md.getMvccPartitionView(null, handle()); + md.getMvccPartitionView(null, handle()); + Assertions.assertEquals(2, loadCount(ops), "a null (disabled) cache must re-enumerate every call"); + } + + // --------------------------------------------------------------------- + // listPartitions + // --------------------------------------------------------------------- + + @Test + public void listPartitionsCachesDerivedListAcrossQueries() { + // WHY: the empty-filter pruning path must memoize the built partition-info list. MUTATION: not consulting + // the cache -> loadTable twice -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache> cache = listCache(); + IcebergConnectorMetadata md = metadataWithListCache(ops, cache); + + List first = md.listPartitions(null, handle(), Optional.empty()); + List second = md.listPartitions(null, handle(), Optional.empty()); + + List names = first.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList()); + Assertions.assertEquals(java.util.Arrays.asList("ts_day=100", "ts_day=101"), names); + Assertions.assertEquals(names, + second.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList())); + Assertions.assertEquals(1, loadCount(ops), "a cache hit must not re-enumerate (loadTable once)"); + } + + @Test + public void listPartitionsWithFilterBypassesCache() { + // WHY: only the empty-filter pruning path is cached; a non-empty filter must BYPASS the cache and compute + // directly (the pruning path always passes Optional.empty()). MUTATION: caching regardless of filter -> + // the second filtered call hits -> loadCount 1 + cache.size 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache> cache = listCache(); + IcebergConnectorMetadata md = metadataWithListCache(ops, cache); + + ConnectorExpression filter = Collections::emptyList; // any non-empty filter + md.listPartitions(null, handle(), Optional.of(filter)); + md.listPartitions(null, handle(), Optional.of(filter)); + Assertions.assertEquals(2, loadCount(ops), "a present filter must bypass the cache (compute every call)"); + // A bypassed call must also NOT populate the cache: a following empty-filter (pruning) call must MISS + // (loadCount 3), not be served from a filtered-populated entry. + md.listPartitions(null, handle(), Optional.empty()); + Assertions.assertEquals(3, loadCount(ops), "the empty-filter call must miss (filtered calls never populate)"); + } + + @Test + public void listPartitionsInvalidateAllForcesReEnumeration() { + // WHY: REFRESH CATALOG (Connector.invalidateAll -> cache.invalidateAll) must drop the cached list. + // MUTATION: invalidateAll not wired -> the second call hits -> loadCount stays 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache> cache = listCache(); + IcebergConnectorMetadata md = metadataWithListCache(ops, cache); + + md.listPartitions(null, handle(), Optional.empty()); + cache.invalidateAll(); + md.listPartitions(null, handle(), Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "invalidateAll must force a re-enumeration"); + } +} diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java index 58df5e326b111d..12c85993a69f94 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java @@ -20,9 +20,11 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorCapability; import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorPartitionInfo; import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorValidationContext; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; import org.apache.doris.connector.metastore.HmsMetaStoreProperties; import org.apache.doris.connector.metastore.spi.JdbcDriverSupport; import org.apache.doris.connector.metastore.spi.MetaStoreProviders; @@ -50,6 +52,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.OptionalLong; @@ -126,6 +129,19 @@ public class PaimonConnector implements Connector { // Cleared wholesale on REFRESH CATALOG (the connector is rebuilt). See PaimonSchemaAtMemo. private final PaimonSchemaAtMemo schemaAtMemo = new PaimonSchemaAtMemo(PaimonSchemaAtMemo.DEFAULT_MAX_SIZE); + // PERF-06: cross-query DERIVED partition-view cache ("cache A", the generic ConnectorPartitionViewCache from + // fe-connector-cache), layered ABOVE the raw remote catalog.listPartitions call (PaimonCatalogOps#listPartitions): + // it memoizes the BUILT List (display-name rendering + null-sentinel normalization, + // see PaimonConnectorMetadata#collectPartitions) keyed by (db, table, snapshotId, schemaId), so a repeated + // query on a partitioned table skips the derived rebuild AND the remote catalog round-trip. ONE typed field + // (unlike iceberg's two): paimon does not override getMvccPartitionView, so the generic MTMV model falls + // back to its default listPartitions/LIST/timestamp path for paimon -- listPartitions is the only + // enumeration hook to wrap. Unlike iceberg, paimon has NO session=user / per-user credential-isolation + // cache-disabling convention (a paimon catalog authenticates at catalog-creation time -- Kerberos UGI / + // HMS principal -- not per-query session identity), so this is constructed unconditionally: never null on + // a live connector (only PaimonConnectorMetadata's convenience/test constructors pass null). + private final ConnectorPartitionViewCache> partitionViewCache; + public PaimonConnector(Map properties, ConnectorContext context) { this.properties = properties; // Wrap the FE-injected context so every executeAuthenticated pins the TCCL to the plugin loader (the @@ -137,6 +153,9 @@ public PaimonConnector(Map properties, ConnectorContext context) this::pluginAuthenticator); this.latestSnapshotCache = new PaimonLatestSnapshotCache(resolveTableCacheTtlSecond(properties), DEFAULT_TABLE_CACHE_CAPACITY); + // Reads its own meta.cache.paimon.partition_view.(enable|ttl-second|capacity) from the catalog + // properties via the framework's CacheSpec (default ON / 24h / 1000). + this.partitionViewCache = new ConnectorPartitionViewCache<>("paimon", properties); } /** @@ -225,7 +244,7 @@ private static long resolveTableCacheTtlSecond(Map properties) { public ConnectorMetadata getMetadata(ConnectorSession session) { return new PaimonConnectorMetadata( new PaimonCatalogOps.CatalogBackedPaimonCatalogOps(ensureCatalog()), properties, context, - schemaAtMemo, latestSnapshotCache); + schemaAtMemo, latestSnapshotCache, partitionViewCache); } @Override @@ -239,6 +258,9 @@ public void invalidateTable(String dbName, String tableName) { // (db,table,sysTable,branch,schemaId) and would otherwise serve a stale schema-at-snapshot after a // drop+recreate that reuses a schemaId (the memo's narrow write-once-per-schemaId assumption breaks). schemaAtMemo.invalidate(dbName, tableName); + // PERF-06: also drop this table's cached derived partition-view entries (every snapshotId cached for + // it), so the next listPartitions re-enumerates live. + partitionViewCache.invalidateTable(dbName, tableName); } /** @@ -255,12 +277,14 @@ public void invalidateTable(String dbName, String tableName) { public void invalidateDb(String dbName) { latestSnapshotCache.invalidateDb(dbName); schemaAtMemo.invalidateDb(dbName); + partitionViewCache.invalidateDb(dbName); } @Override public void invalidateAll() { latestSnapshotCache.invalidateAll(); schemaAtMemo.invalidateAll(); + partitionViewCache.invalidateAll(); } @Override @@ -316,6 +340,11 @@ public Set getCapabilities() { ConnectorCapability.SUPPORTS_SHOW_CREATE_DDL); } + /** Test-only: the derived listPartitions view cache (PERF-06). Never null (paimon has no session=user gate). */ + ConnectorPartitionViewCache> partitionViewCacheForTest() { + return partitionViewCache; + } + private Catalog ensureCatalog() { if (catalog == null) { synchronized (this) { diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java index c2e1fe021da017..bd59897e3f03ec 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java @@ -32,6 +32,8 @@ import org.apache.doris.connector.api.mvcc.ConnectorTimeTravelSpec; import org.apache.doris.connector.api.pushdown.ConnectorExpression; import org.apache.doris.connector.api.scan.ConnectorPartitionValues; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.thrift.THiveTable; import org.apache.doris.thrift.TTableDescriptor; @@ -95,6 +97,15 @@ public class PaimonConnectorMetadata implements ConnectorMetadata { // existing direct-construction tests compile unchanged; production goes through the 5-arg ctor. private final PaimonLatestSnapshotCache latestSnapshotCache; + // PERF-06: cross-query DERIVED partition-view cache A (generic ConnectorPartitionViewCache), injected by the + // owning PaimonConnector; null = no cross-query derived layer (the convenience/test ctors used by ~15 + // existing direct-construction tests pass null). Layered ABOVE the raw remote catalogOps.listPartitions + // call: a hit skips both the derived-view BUILD (collectPartitions) and the remote round-trip, keyed by + // (db, table, snapshotId, schemaId). Consumed only by listPartitions -- paimon does not override + // getMvccPartitionView (see ConnectorMetadata's default), so the generic MTMV model already uses + // listPartitions for its LIST/timestamp partition view; there is no second hook to wrap. + private final ConnectorPartitionViewCache> partitionViewCache; + public PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map properties, ConnectorContext context) { this(catalogOps, properties, context, new PaimonSchemaAtMemo(PaimonSchemaAtMemo.DEFAULT_MAX_SIZE)); @@ -105,15 +116,30 @@ public PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map this(catalogOps, properties, context, schemaAtMemo, new PaimonLatestSnapshotCache(0L, 1)); } + /** Convenience ctor without the PERF-06 derived partition-view cache (null -> listPartitions always live). */ PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map properties, ConnectorContext context, PaimonSchemaAtMemo schemaAtMemo, PaimonLatestSnapshotCache latestSnapshotCache) { + this(catalogOps, properties, context, schemaAtMemo, latestSnapshotCache, null); + } + + /** + * Full ctor used by {@link PaimonConnector#getMetadata}, adding the PERF-06 derived partition-view cache + * (cache A): {@code partitionViewCache} memoizes {@link #listPartitions}'s built + * {@code List}, keyed by {@code (db, table, snapshotId, schemaId)}. {@code null} + * for the convenience/test ctors (no cross-query derived layer -> compute directly every call). + */ + PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map properties, + ConnectorContext context, PaimonSchemaAtMemo schemaAtMemo, + PaimonLatestSnapshotCache latestSnapshotCache, + ConnectorPartitionViewCache> partitionViewCache) { this.catalogOps = catalogOps; this.typeMappingOptions = buildTypeMappingOptions(properties); this.context = context; this.catalogProperties = properties; this.schemaAtMemo = schemaAtMemo; this.latestSnapshotCache = latestSnapshotCache; + this.partitionViewCache = partitionViewCache; } @Override @@ -955,11 +981,57 @@ public List listPartitionNames(ConnectorSession session, ConnectorTableH * {@code PaimonExternalCatalog.getPaimonPartitions} returns the full partition set without * pushing predicates into the Paimon catalog, and this preserves that behavior (mirrors * {@code MaxComputeConnectorMetadata}). + * + *

PERF-06 cache A: the BUILT {@code List} is memoized across queries in + * {@link #partitionViewCache}, keyed by {@code (db, table, snapshotId, schemaId)} (see + * {@link #partitionViewCacheKey}) — a hit skips both {@link #collectPartitions} and the remote + * {@code catalogOps.listPartitions} round-trip. The cache is BYPASSED (compute directly, never populated) + * when: {@code partitionViewCache} is {@code null} (the convenience/test ctors); {@code filter} is present + * (not the pruning path, and not keyed by filter); or the handle is unpartitioned (mirrors + * {@link #collectPartitions}'s own empty-partitionKeys short-circuit, so an unpartitioned table never + * touches {@link #latestSnapshotCache} either — preserving the "no seam call" contract the unpartitioned + * path already guarantees). */ @Override public List listPartitions(ConnectorSession session, ConnectorTableHandle handle, Optional filter) { - return collectPartitions((PaimonTableHandle) handle); + PaimonTableHandle paimonHandle = (PaimonTableHandle) handle; + List partitionKeys = paimonHandle.getPartitionKeys(); + if (partitionViewCache == null || filter.isPresent() + || partitionKeys == null || partitionKeys.isEmpty()) { + return collectPartitions(paimonHandle); + } + PartitionViewCacheKey key = partitionViewCacheKey(paimonHandle); + return partitionViewCache.get(key, () -> collectPartitions(paimonHandle)); + } + + /** + * Builds cache A's key for {@code paimonHandle}: {@code (db, table, snapshotId, schemaId)}. + * + *

snapshotId: {@link #collectPartitions}'s remote call ({@code catalogOps.listPartitions(Identifier)}) + * is BASE-identifier-only — it does not apply the handle's pinned {@code scanOptions} (unlike the scan path), + * so it always reflects the CURRENT catalog state, never a time-travel pin (branch / time-travel reads never + * reach this path at all — see {@link #collectPartitions}). The key must therefore track "current", not + * whatever snapshot happens to be threaded on the handle: it reads the SAME per-catalog + * {@link #latestSnapshotCache} that {@link #beginQuerySnapshot} pins queries to (a cheap in-memory hit within + * the query — {@code beginQuerySnapshot} already warmed it), so a repeat query within the TTL hits this cache, + * and a new snapshot (data change, once the entry expires or REFRESH invalidates it) naturally mints a new key. + * + *

schemaId: pinned {@code -1} ("unversioned" for that axis, matching + * {@link PartitionViewCacheKey}'s documented convention). Unlike iceberg, paimon's {@link PaimonTableHandle} + * carries no schemaId — {@code applySnapshot} threads only {@code scanOptions} (an opaque properties map; + * see its javadoc) onto the handle, and {@link #beginQuerySnapshot} (the common latest-pin path) never + * resolves a schemaId either (its {@code ConnectorMvccSnapshot} keeps the builder default {@code -1}). This + * is not a loss for THIS view: {@link #collectPartitions} derives its output from {@code partitionKeys} + * (fixed at handle-build time) and paimon's raw partition specs, and paimon partition columns are immutable + * post-creation, so schema evolution (e.g. ADD COLUMN) does not change what this method computes. + */ + private PartitionViewCacheKey partitionViewCacheKey(PaimonTableHandle paimonHandle) { + Identifier identifier = Identifier.create(paimonHandle.getDatabaseName(), paimonHandle.getTableName()); + long snapshotId = latestSnapshotCache.getOrLoad(identifier, + () -> catalogOps.latestSnapshotId(resolveTable(paimonHandle)).orElse(-1L)); + return new PartitionViewCacheKey( + paimonHandle.getDatabaseName(), paimonHandle.getTableName(), snapshotId, -1L); } @Override diff --git a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java index 7e7aa033229238..f952e6d47a7def 100644 --- a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java +++ b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java @@ -17,13 +17,19 @@ package org.apache.doris.connector.paimon; +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.OptionalLong; +import java.util.function.Supplier; /** * Tests PaimonConnector's FIX-4 cache knobs (CI 973411): the {@code meta.cache.paimon.table.ttl-second} @@ -83,4 +89,64 @@ public void invalidateHooksAreNoThrowOnFreshConnector() { Assertions.assertDoesNotThrow(() -> connector.invalidateDb("db1")); Assertions.assertDoesNotThrow(connector::invalidateAll); } + + // ============ PERF-06: derived partition-view cache A (no session=user gate) + invalidation ============ + + @Test + public void partitionViewCacheAlwaysBuilt() { + // Unlike iceberg, paimon has NO session=user / per-user credential-isolation cache-disabling + // convention (a paimon catalog authenticates at catalog-creation time, not per-query session + // identity), so the connector must construct cache A unconditionally on every flavor. MUTATION: a + // stray session-like gate leaving the field null on some property combination -> red. + Assertions.assertNotNull(connector(Collections.emptyMap()).partitionViewCacheForTest(), + "a fresh paimon connector must always build the partition-view cache"); + Map withTtl = props("3600"); + Assertions.assertNotNull(connector(withTtl).partitionViewCacheForTest(), + "the partition-view cache is independent of meta.cache.paimon.table.ttl-second"); + } + + @Test + public void refreshHooksInvalidatePartitionViewCache() { + // The REFRESH hooks must clear cache A too (else external DDL/writes stay invisible beyond the TTL): + // REFRESH TABLE drops that table's snapshot entries, REFRESH DATABASE that db's, REFRESH CATALOG + // everything. Asserted via a counting loader (the framework's size() is package-private): after + // invalidation the loader must run again. MUTATION: an invalidate* hook not routed to the view cache + // -> the entry survives -> loader not re-run -> a loads assert below red. + PaimonConnector connector = connector(Collections.emptyMap()); + ConnectorPartitionViewCache> cache = connector.partitionViewCacheForTest(); + Assertions.assertNotNull(cache); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", 1L, -1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", 1L, -1L); + PartitionViewCacheKey db2t1 = new PartitionViewCacheKey("db2", "t1", 1L, -1L); + + // REFRESH TABLE db1.t1 -> only db1.t1 re-loads. + cache.get(db1t1, loader); + cache.get(db1t1, loader); + Assertions.assertEquals(1, loads[0], "second get is a hit"); + connector.invalidateTable("db1", "t1"); + cache.get(db1t1, loader); + Assertions.assertEquals(2, loads[0], "REFRESH TABLE forces a reload of db1.t1"); + + // REFRESH DATABASE db1 -> db1.t2 re-loads; db2.t1 unaffected. + cache.get(db1t2, loader); // loads=3 (miss) + cache.get(db2t1, loader); // loads=4 (miss) + cache.get(db1t2, loader); // hit + cache.get(db2t1, loader); // hit + Assertions.assertEquals(4, loads[0]); + connector.invalidateDb("db1"); + cache.get(db2t1, loader); // db2 untouched -> hit + Assertions.assertEquals(4, loads[0], "REFRESH DATABASE db1 must NOT drop db2's entries"); + cache.get(db1t2, loader); // db1.t2 dropped -> miss + Assertions.assertEquals(5, loads[0], "REFRESH DATABASE db1 drops db1's entries"); + + // REFRESH CATALOG -> everything re-loads. + connector.invalidateAll(); + cache.get(db2t1, loader); + Assertions.assertEquals(6, loads[0], "REFRESH CATALOG drops everything"); + } } diff --git a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java new file mode 100644 index 00000000000000..e93126549df16d --- /dev/null +++ b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java @@ -0,0 +1,262 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.paimon; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; + +import org.apache.paimon.partition.Partition; +import org.apache.paimon.types.DataTypes; +import org.apache.paimon.types.RowType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalLong; +import java.util.stream.Collectors; + +/** + * PERF-06 tests for the cross-query DERIVED partition-view cache ("cache A", the generic + * {@link ConnectorPartitionViewCache}) wired into {@link PaimonConnectorMetadata#listPartitions}. Paimon does + * NOT override {@code getMvccPartitionView} (the generic MTMV model falls back to its default + * listPartitions/LIST/timestamp path), so — unlike iceberg's two typed fields — there is exactly ONE + * enumeration hook to wrap. + * + *

Uses the real {@link RecordingPaimonCatalogOps} + {@link FakePaimonTable} harness (no Mockito, no docker): + * {@link PaimonConnectorMetadata#collectPartitions}'s first (and only) remote call is + * {@code catalogOps.listPartitions(Identifier)}, logged as {@code "listPartitions:db1.t1"}, so a cache HIT skips + * the whole loader and that log count is the enumeration counter — the SAME proxy + * {@link PaimonConnectorMetadataPartitionTest} already relies on ({@code ops.log.contains("listPartitions:...")}). + * + *

Every test uses a DISABLED {@link PaimonLatestSnapshotCache} ({@code ttl-second <= 0}, always-live) so the + * cache-A key's {@code snapshotId} component is driven directly and deterministically by + * {@code ops.latestSnapshotId}, isolating cache A (the only caching layer under test) — mirroring how the + * iceberg PERF-06 tests pass the raw partition cache as {@code null} throughout. + */ +public class PaimonConnectorMetadataPartitionViewCacheTest { + + private static PaimonConnectorMetadata metadataWithCache(RecordingPaimonCatalogOps ops, + ConnectorPartitionViewCache> cache) { + return new PaimonConnectorMetadata(ops, Collections.emptyMap(), new RecordingConnectorContext(), + new PaimonSchemaAtMemo(PaimonSchemaAtMemo.DEFAULT_MAX_SIZE), + new PaimonLatestSnapshotCache(0L, 1), cache); + } + + private static ConnectorPartitionViewCache> partitionViewCache() { + return new ConnectorPartitionViewCache<>("paimon", Collections.emptyMap()); + } + + private static RowType regionRowType() { + return RowType.builder() + .field("id", DataTypes.INT()) + .field("region", DataTypes.STRING()) + .build(); + } + + private static PaimonTableHandle handle(FakePaimonTable table) { + PaimonTableHandle h = new PaimonTableHandle( + "db1", "t1", Collections.singletonList("region"), Collections.emptyList()); + h.setPaimonTable(table); + return h; + } + + private static FakePaimonTable regionTable() { + FakePaimonTable table = new FakePaimonTable( + "t1", regionRowType(), Collections.singletonList("region"), Collections.emptyList()); + table.setOptions(Collections.singletonMap("partition.legacy-name", "true")); + return table; + } + + private static Partition partition(String regionValue) { + Map spec = new LinkedHashMap<>(); + spec.put("region", regionValue); + return new Partition(spec, 1L, 1L, 1, 1L, true); + } + + private static long loadCount(RecordingPaimonCatalogOps ops) { + return ops.log.stream().filter(s -> s.equals("listPartitions:db1.t1")).count(); + } + + private static List names(List infos) { + return infos.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList()); + } + + @Test + public void listPartitionsCachesDerivedListAcrossQueries() { + // WHY: cache A must memoize the BUILT List keyed by (db, table, snapshotId, + // schemaId), so a repeated query on the same (unchanged) latest snapshot skips the derived rebuild AND + // the remote catalogOps.listPartitions round-trip. MUTATION: not consulting the cache (compute directly + // every call) -> the seam runs twice -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Arrays.asList(partition("cn"), partition("us")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + List first = md.listPartitions(null, h, Optional.empty()); + List second = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertEquals(Arrays.asList("region=cn", "region=us"), names(first)); + Assertions.assertEquals(names(first), names(second), "the cached list is returned verbatim"); + Assertions.assertEquals(1, loadCount(ops), "a cache hit must not re-enumerate (listPartitions once)"); + } + + @Test + public void listPartitionsDifferentSnapshotReEnumerates() { + // WHY: the underlying remote enumeration always reflects the CURRENT catalog state (it is + // base-identifier-only, never pinned to a historical snapshot -- see partitionViewCacheKey's javadoc), + // so the key must track "current" via latestSnapshotCache: a data change (new latest snapshot id) + // must mint a new key and re-enumerate. MUTATION: keying on (db,table) only, ignoring snapshotId -> + // the stale S1 list would be served after the snapshot changed -> loadCount/contents below red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + List atS1 = md.listPartitions(null, h, Optional.empty()); + + ops.latestSnapshotId = OptionalLong.of(200L); + ops.partitions = Arrays.asList(partition("cn"), partition("us")); + List atS2 = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertEquals(Collections.singletonList("region=cn"), names(atS1)); + Assertions.assertEquals(Arrays.asList("region=cn", "region=us"), names(atS2)); + Assertions.assertEquals(2, loadCount(ops), "distinct snapshot keys must each enumerate"); + } + + @Test + public void listPartitionsInvalidateTableForcesReEnumeration() { + // WHY: REFRESH TABLE (PaimonConnector.invalidateTable -> cache.invalidateTable) must drop the cached + // list so the next query re-enumerates live. MUTATION: invalidateTable not wired -> the second call + // hits the stale entry -> loadCount stays 1 -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateTable("db1", "t1"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "invalidateTable must force a re-enumeration"); + } + + @Test + public void listPartitionsInvalidateAllForcesReEnumeration() { + // WHY: REFRESH CATALOG (PaimonConnector.invalidateAll -> cache.invalidateAll) must drop the cached + // list. MUTATION: invalidateAll not wired -> the second call hits -> loadCount stays 1 -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateAll(); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "invalidateAll must force a re-enumeration"); + } + + @Test + public void listPartitionsWithFilterBypassesCache() { + // WHY: a present filter must BYPASS the cache and compute directly -- and must NOT populate it either, + // so a later empty-filter (pruning) call still misses. legacy ignores the filter value entirely + // (returns the full set regardless), but the cache-A key carries no filter dimension, so caching a + // filtered call's result would be indistinguishable from caching an unfiltered one; bypassing keeps the + // two paths independent, mirroring the iceberg pattern. MUTATION: caching regardless of filter -> the + // second filtered call hits (loadCount stays 1) -> red; or a bypassed call populating the cache -> the + // following empty-filter call hits instead of missing -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + ConnectorExpression filter = Collections::emptyList; // any non-empty filter + md.listPartitions(null, h, Optional.of(filter)); + md.listPartitions(null, h, Optional.of(filter)); + Assertions.assertEquals(2, loadCount(ops), "a present filter must bypass the cache (compute every call)"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(3, loadCount(ops), "the empty-filter call must miss (filtered calls never populate)"); + } + + @Test + public void listPartitionsNullCacheEnumeratesEveryCall() { + // The convenience/test-ctor analogue: a null cache means compute directly every call -> the seam runs + // on every query (no cross-query sharing). MUTATION: defaulting to a shared cache when null was + // intended -> loadCount 1 -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + PaimonConnectorMetadata md = metadataWithCache(ops, null); + PaimonTableHandle h = handle(table); + + md.listPartitions(null, h, Optional.empty()); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "a null (disabled) cache must re-enumerate every call"); + } + + @Test + public void unpartitionedHandleBypassesCacheWithoutTouchingSnapshotSeam() { + // WHY: an unpartitioned handle must short-circuit to empty WITHOUT touching either seam + // (latestSnapshotId or listPartitions) -- mirrors collectPartitions' own pre-existing contract + // (PaimonConnectorMetadataPartitionTest#nonPartitionedHandleReturnsEmptyWithoutSeamCall). Building a + // cache-A key would otherwise call latestSnapshotCache -> catalogOps.latestSnapshotId for a table that + // is guaranteed to return an empty list, polluting the cache and the seam log for nothing. MUTATION: + // building the key before the emptiness check -> "latestSnapshotId" appears in ops.log -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = new FakePaimonTable( + "t1", RowType.builder().field("id", DataTypes.INT()).build(), + Collections.emptyList(), Collections.emptyList()); + ops.table = table; + PaimonTableHandle h = new PaimonTableHandle( + "db1", "t1", Collections.emptyList(), Collections.emptyList()); + h.setPaimonTable(table); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + + List result = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertTrue(result.isEmpty()); + Assertions.assertTrue(ops.log.isEmpty(), "an unpartitioned handle must not touch any remote seam"); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java index e1ff655002044e..115fb6becb0d0a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java @@ -20,6 +20,7 @@ import org.apache.doris.catalog.Env; import org.apache.doris.common.Config; import org.apache.doris.common.ThreadPoolManager; +import org.apache.doris.common.cache.NereidsSortedPartitionsCacheManager; import org.apache.doris.datasource.doris.DorisExternalMetaCache; import org.apache.doris.datasource.metacache.AbstractExternalMetaCache; import org.apache.doris.datasource.metacache.ExternalMetaCache; @@ -184,6 +185,10 @@ public void invalidateCatalog(long catalogId) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "invalidateCatalog", () -> cache.invalidateCatalogEntries(catalogId))); + // Cache B (Nereids sorted-partition-ranges) has no db/catalog-scoped eviction, so a catalog-level + // REFRESH must drop ALL of its entries -- else binary-search pruning could serve ranges older than + // the refreshed metadata. Coarse but correct (a rebuild is cheap and lazy). Mirrors invalidateTable. + invalidateSortedPartitionsCache(); } public void invalidateCatalogByEngine(long catalogId, String engine) { @@ -196,6 +201,9 @@ public void removeCatalog(long catalogId) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "removeCatalog", () -> cache.invalidateCatalog(catalogId))); + // Drop ALL Cache B entries: the catalog is going away and Cache B has no catalog-scoped eviction. + // (invalidateAll needs no catalog name, so this is safe even after the catalog was already removed.) + invalidateSortedPartitionsCache(); } public void removeCatalogByEngine(long catalogId, String engine) { @@ -207,12 +215,37 @@ public void removeCatalogByEngine(long catalogId, String engine) { public void invalidateDb(long catalogId, String dbName) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "invalidateDb", () -> cache.invalidateDb(catalogId, dbName))); + // Cache B has no db-scoped eviction key, so a db-level REFRESH drops ALL entries (coarse but + // correct -- a rebuild is cheap and lazy). Mirrors invalidateTable's Cache B wiring. + invalidateSortedPartitionsCache(); } public void invalidateTable(long catalogId, String dbName, String tableName) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "invalidateTable", () -> cache.invalidateTable(catalogId, dbName, tableName))); + // Also drop the Nereids sorted-partition-ranges cache for this external table so binary-search + // pruning does not serve ranges older than the refreshed metadata. + CatalogIf ctl = getCatalog(catalogId); + if (ctl != null) { + Env.getCurrentEnv().getSortedPartitionsCacheManager().invalidateTable(ctl.getName(), dbName, tableName); + } + } + + /** + * Drops ALL entries of the Nereids sorted-partition-ranges cache (Cache B). Used by the db/catalog-level + * invalidations, which have no finer-grained (db/catalog-scoped) eviction key on that cache. Null-safe: + * during early startup / checkpoint replay {@code Env.getCurrentEnv()} or its cache manager may be unset. + */ + private void invalidateSortedPartitionsCache() { + Env env = Env.getCurrentEnv(); + if (env == null) { + return; + } + NereidsSortedPartitionsCacheManager mgr = env.getSortedPartitionsCacheManager(); + if (mgr != null) { + mgr.invalidateAll(); + } } public void invalidateTableByEngine(long catalogId, String engine, String dbName, String tableName) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java index ade57ee4262db6..94010de31d700f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java @@ -21,11 +21,13 @@ import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; import org.apache.doris.catalog.TableAttributes; import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.TableIndexes; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Pair; +import org.apache.doris.common.cache.NereidsSortedPartitionsCacheManager; import org.apache.doris.common.io.Text; import org.apache.doris.common.io.Writable; import org.apache.doris.common.util.PropertyAnalyzer; @@ -517,15 +519,22 @@ public boolean supportInternalPartitionPruned() { } /** - * Get sorted partition ranges for binary search filtering. - * Subclasses can override this method to provide sorted partition ranges - * for efficient partition pruning. + * Cross-query cache of the pre-built {@link SortedPartitionRanges} for binary-search partition + * pruning. Tables that implement {@link SupportBinarySearchFilteringPartitions} (external MVCC: + * iceberg/paimon) route through the shared {@link NereidsSortedPartitionsCacheManager}, keyed by the + * pinned connector snapshot; others return empty (the caller falls back to building ranges fresh). * * @param scan the catalog relation * @return sorted partition ranges, or empty if not supported */ + @SuppressWarnings({"unchecked", "rawtypes"}) public Optional> getSortedPartitionRanges(CatalogRelation scan) { - return Optional.empty(); + if (!(this instanceof SupportBinarySearchFilteringPartitions)) { + return Optional.empty(); + } + Optional> cached = Env.getCurrentEnv().getSortedPartitionsCacheManager() + .get((SupportBinarySearchFilteringPartitions) this, scan); + return (Optional) cached; } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java index bb3edc19f44fad..41bd5ad2b618de 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java @@ -27,6 +27,7 @@ import org.apache.doris.catalog.PartitionKey; import org.apache.doris.catalog.PartitionType; import org.apache.doris.catalog.RangePartitionItem; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; import org.apache.doris.catalog.Type; import org.apache.doris.common.AnalysisException; import org.apache.doris.connector.api.Connector; @@ -54,6 +55,8 @@ import org.apache.doris.mtmv.MTMVSnapshotIdSnapshot; import org.apache.doris.mtmv.MTMVSnapshotIf; import org.apache.doris.mtmv.MTMVTimestampSnapshot; +import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -64,6 +67,7 @@ import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -87,7 +91,7 @@ * degrades MTMV to conservative over-refresh (the partition never matches its prior snapshot).

*/ public class PluginDrivenMvccExternalTable extends PluginDrivenExternalTable - implements MTMVRelatedTableIf, MTMVBaseTableIf, MvccTable { + implements MTMVRelatedTableIf, MTMVBaseTableIf, MvccTable, SupportBinarySearchFilteringPartitions { private static final Logger LOG = LogManager.getLogger(PluginDrivenMvccExternalTable.class); @@ -619,6 +623,58 @@ public Set getPartitionColumnNames(Optional snapshot) { .map(c -> c.getName().toLowerCase()).collect(Collectors.toSet()); } + // ── SupportBinarySearchFilteringPartitions: lets NereidsSortedPartitionsCacheManager cache the + // pre-built SortedPartitionRanges across queries, keyed by the pinned connector snapshot. ── + @Override + public Map getOriginPartitions(CatalogRelation scan) { + // The SAME frozen map the pin exposes — no re-list, so ranges stay consistent with + // nameToPartitionItem (no #65659 TOCTOU). + return getNameToPartitionItems(pinnedSnapshot(scan)); + } + + @Override + public Object getPartitionMetaVersion(CatalogRelation scan) { + // The version token MUST equal the EXACT partition content the ranges are built from + // (getOriginPartitions reads the SAME pin), so Cache B can never disagree with the pin's own + // nameToPartitionItem regardless of how/when a connector's listPartitions cache (Cache A) + // refreshes: the cache rebuilds precisely when the partition set changes, never on a stale set. + // A compact copy of just the name strings (NOT the live keySet view), so the cross-query cache + // entry does not retain the whole pin's partition-item map. + // + // The former "@" O(1) token was REMOVED: it is only sound where a + // connector's listPartitions content is a pure function of the snapshot id, which is NOT true for + // iceberg NON-RANGE tables (identity / bucket / truncate / multi-field partitioning). There the + // pin's nameToPartitionItem is served by listPartitions (Cache A, keyed (-1,-1), enumerated at + // currentSnapshot with its own TTL, ignoring the pin) while the snapshot-id token comes from a + // DIFFERENT cache (IcebergLatestSnapshotCache) with its own TTL; the two expire independently, so + // a snapshot-id token could serve SortedPartitionRanges STALER than the pin's own partitions -> + // within-query disagreement -> silent under-inclusive pruning. Deriving the version from the + // frozen name set makes it uniform across ALL engines -- the same rigorous scheme snapshot-less + // hive already relied on -- at the cost of an O(partitions) set copy per lookup. + return new HashSet<>(getOriginPartitions(scan).keySet()); + } + + @Override + public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { + // No insert-frequency signal for external tables; 0 = always allow sorting (the manager's + // "skip sort if loaded within cacheSortedPartitionIntervalSecond" heuristic never trips). + return 0L; + } + + // getSortedPartitionRanges is now a pure pass-through to ExternalTable's cache-manager delegation + // (5c17b748880's snapshotId==-1 short-circuit was removed: getPartitionMetaVersion above derives a + // content-comparable version from the frozen partition name set for ALL engines, so Cache B is + // correct uniformly -- snapshot-less hive and real-snapshot iceberg/paimon alike) -- no override + // needed here. + + private Optional pinnedSnapshot(CatalogRelation scan) { + if (scan instanceof LogicalFileScan) { + LogicalFileScan fileScan = (LogicalFileScan) scan; + return MvccUtil.getSnapshotFromContext(this, fileScan.getTableSnapshot(), fileScan.getScanParams()); + } + return MvccUtil.getSnapshotFromContext(this); + } + // ──────────────────── MTMV snapshots ──────────────────── @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java index ad09a5307e03a1..f53530c2837561 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java @@ -99,6 +99,7 @@ private SelectedPartitions pruneExternalPartitions(ExternalTable externalTable, || ctx.getConnectContext().getSessionVariable().enableBinarySearchFilteringPartitions; if (enableBinarySearch && !nameToPartitionItem.isEmpty()) { sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges + .or(() -> (Optional) externalTable.getSortedPartitionRanges(scan)) .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))); } PartitionPruneResult result = PartitionPruner.pruneWithResult( @@ -108,9 +109,12 @@ private SelectedPartitions pruneExternalPartitions(ExternalTable externalTable, for (String name : prunedPartitions) { PartitionItem item = nameToPartitionItem.get(name); - // Both nameToPartitionItem and sortedPartitionRanges now come from the same frozen - // snapshot, so a missing item is an invariant violation rather than a partition to - // skip. Failing here surfaces the bug instead of silently returning a partial scan. + // Within THIS query, nameToPartitionItem and sortedPartitionRanges are built from the same + // frozen map. On a cross-query cache HIT, sortedPartitionRanges instead reuses ranges built by + // an earlier query keyed by the identical (snapshotId, schemaId) version token -- content is + // identical only via the MVCC determinism premise (same version token => same partition set), + // not because the two maps are literally the same object. A missing item here means that + // premise was violated, so fail loud rather than silently returning a partial scan. Preconditions.checkState(item != null, "pruned partition %s is missing in the selected partitions snapshot", name); selectedPartitionItems.put(name, item); diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java new file mode 100644 index 00000000000000..7e810f54a00c9d --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -0,0 +1,581 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.common.cache; + +import org.apache.doris.analysis.PartitionValue; +import org.apache.doris.analysis.TableScanParams; +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.ListPartitionItem; +import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.PartitionKey; +import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; +import org.apache.doris.connector.api.mvcc.ConnectorMvccSnapshot; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.CatalogMgr; +import org.apache.doris.datasource.ExternalDatabase; +import org.apache.doris.datasource.ExternalMetaCacheMgr; +import org.apache.doris.datasource.ExternalTable; +import org.apache.doris.datasource.mvcc.PluginDrivenMvccExternalTable; +import org.apache.doris.datasource.mvcc.PluginDrivenMvccSnapshot; +import org.apache.doris.nereids.StatementContext; +import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; +import org.apache.doris.nereids.trees.plans.RelationId; +import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; +import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan.SelectedPartitions; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.rpc.RpcException; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.Map; +import java.util.Optional; + +/** + * Unit coverage for wiring external MVCC tables (iceberg/paimon) into + * {@link NereidsSortedPartitionsCacheManager}: cache hit/miss/rebuild-on-version-change, origin-map + * consistency (no #65659 TOCTOU), invalidation, and the {@link ExternalTable#getSortedPartitionRanges} + * delegation contract. + * + *

Drives the manager with a Mockito mock of {@link SupportBinarySearchFilteringPartitions} rather than + * a hand-written fake class: the interface extends {@code TableIf}, whose large method surface makes a + * hand-rolled implementer impractical. Only the methods the manager actually calls are stubbed + * ({@code getOriginPartitions}, {@code getPartitionMetaVersion}, {@code getPartitionMetaLoadTimeMillis}, + * {@code getId}, {@code getName}, {@code getDatabase}).

+ * + *

{@link NereidsSortedPartitionsCacheManager#get} dereferences {@code ConnectContext.get() + * .getSessionVariable()} unconditionally once {@code ConnectContext.get() != null}, so every test needs a + * live {@link ConnectContext} (mirrors the lightweight idiom in {@code LogicalFileScanTest}: a plain + * {@code new ConnectContext()} + {@code setThreadLocalInfo()}, no FE server bootstrap).

+ * + *

The two tests at the bottom of this file additionally drive the REAL production wiring the + * FakeExternalTable-based tests above bypass: {@code PluginDrivenMvccExternalTable#getOriginPartitions} + * / {@code #getPartitionMetaVersion} / {@code #pinnedSnapshot} (via a {@code CALLS_REAL_METHODS} mock, + * the same technique as {@code LogicalFileScanTest}), and {@code ExternalMetaCacheMgr#invalidateTable}'s + * call into this manager (via a real {@link NereidsSortedPartitionsCacheManager} instance reached through + * a mocked {@code Env}).

+ */ +public class NereidsSortedPartitionsCacheManagerExternalTest { + + private static final String CTL = "ctl"; + private static final String DB = "db"; + private static final String TBL = "t"; + private static final long CATALOG_ID = 7L; + + @AfterEach + public void tearDown() { + ConnectContext.remove(); + } + + private static void newLiveConnectContext() { + ConnectContext ctx = new ConnectContext(); + ctx.setThreadLocalInfo(); + } + + private static ListPartitionItem listItem(int value) throws Exception { + Column partitionColumn = new Column("id", PrimitiveType.INT); + PartitionValue partitionValue = new PartitionValue(String.valueOf(value)); + PartitionKey partitionKey = PartitionKey.createPartitionKey( + ImmutableList.of(partitionValue), ImmutableList.of(partitionColumn)); + return new ListPartitionItem(ImmutableList.of(partitionKey)); + } + + /** + * A settable-state mock of {@link SupportBinarySearchFilteringPartitions}: {@link #version} and + * {@link #parts} drive the cache manager's hit/rebuild decision; when {@code withDatabase} is true the + * constructor also stubs a database/catalog pair (names {@link #CTL}/{@link #DB}) so + * {@code TableIdentifier} can build. + */ + private static final class FakeExternalTable { + final SupportBinarySearchFilteringPartitions table = Mockito.mock(SupportBinarySearchFilteringPartitions.class); + Object version = "s1@0"; + Map parts = Maps.newHashMap(); + + @SuppressWarnings({"unchecked", "rawtypes"}) + FakeExternalTable(boolean withDatabase) throws RpcException { + if (withDatabase) { + DatabaseIf db = Mockito.mock(DatabaseIf.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.when(db.getFullName()).thenReturn(DB); + Mockito.when(db.getCatalog()).thenReturn(catalog); + Mockito.when(table.getDatabase()).thenReturn(db); + } + Mockito.when(table.getId()).thenReturn(1001L); + Mockito.when(table.getName()).thenReturn(TBL); + Mockito.when(table.getOriginPartitions(Mockito.any())).thenAnswer(inv -> parts); + Mockito.when(table.getPartitionMetaVersion(Mockito.any())).thenAnswer(inv -> version); + Mockito.when(table.getPartitionMetaLoadTimeMillis(Mockito.any())).thenReturn(0L); + } + } + + // ──────────────────── Task 1: getDatabase()==null guards the external wiring contract ──────────────────── + + @Test + public void testManagerReturnsEmptyWhenDatabaseNull() throws Exception { + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(false); // getDatabase() -> null (unstubbed mock default) + + Optional> r = mgr.get(t.table, (CatalogRelation) null); + + Assertions.assertFalse(r.isPresent(), + "manager must return empty when getDatabase()==null (guards the external wiring contract)"); + } + + // ──────────────────── Task 2: ExternalTable.getSortedPartitionRanges delegation ──────────────────── + + @Test + public void testGetSortedPartitionRangesEmptyForNonSupportTable() { + // A plain ExternalTable does not implement SupportBinarySearchFilteringPartitions, so the + // delegation must short-circuit to empty WITHOUT touching Env/the cache manager. + ExternalTable table = new ExternalTable(); + Assertions.assertFalse(table.getSortedPartitionRanges(null).isPresent(), + "base ExternalTable (not Support) yields empty"); + } + + // ──────────────────── Task 4: invalidate is safe on an absent key ──────────────────── + + @Test + public void testInvalidateEvictsRanges() { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + Assertions.assertEquals(0, mgr.getPartitionCaches().estimatedSize(), + "fresh manager is empty; invalidate is a no-op that must not throw"); + Assertions.assertDoesNotThrow(() -> mgr.invalidateTable(CTL, DB, TBL), + "invalidateTable on an absent key must not throw"); + } + + // ──────────────────── Task 5: cache hit / version-rebuild / origin-map consistency ──────────────────── + + @Test + public void testCacheHitThenRebuildOnVersionChange() throws Exception { + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + + t.version = "s1@0"; + SortedPartitionRanges first = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at snapshot s1"); + SortedPartitionRanges hit = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertSame(first, hit, "same snapshot => cache hit returns the SAME instance"); + + t.version = "s2@0"; // snapshot advanced (ALTER ADD PARTITION) + t.parts.put("id=3", listItem(3)); + SortedPartitionRanges rebuilt = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotSame(first, rebuilt, "version change => rebuild"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + + // Task 4 wiring: dropping the cache by (catalog, db, table) forces the next get() to rebuild too. + mgr.invalidateTable(CTL, DB, TBL); + SortedPartitionRanges afterInvalidate = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotSame(rebuilt, afterInvalidate, + "explicit invalidateTable(catalog, db, table) forces a rebuild on the next get()"); + } + + @Test + public void testRangesConsistentWithOriginMap() throws Exception { + // The cached ranges are built from getOriginPartitions(scan); every range id must be a key of + // that same map -- the invariant PruneFileScanPartition's Preconditions relies on (no TOCTOU). + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + + SortedPartitionRanges r = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotNull(r); + r.sortedPartitions.forEach(p -> + Assertions.assertTrue(t.parts.containsKey(p.id), "every range id is a key of the origin map")); + } + + // ──────────────────── Real production wiring: PluginDrivenMvccExternalTable ──────────────────── + // + // The tests above drive the manager with a hand-stubbed SupportBinarySearchFilteringPartitions mock + // and never touch PluginDrivenMvccExternalTable, so they miss the NEW wiring in + // getOriginPartitions/getPartitionMetaVersion/pinnedSnapshot (PluginDrivenMvccExternalTable.java + // around :628-651). This test drives those REAL method bodies: Mockito.CALLS_REAL_METHODS runs every + // unstubbed method for real, so only the connector round-trip (loadSnapshot) and the identity fields + // MvccTableInfo needs (getName/getDatabase) are stubbed -- mirroring the technique in + // LogicalFileScanTest#computeOutputBindsThisReferencesOwnVersionNotLatest. + + @Test + public void testPluginDrivenMvccExternalTableRealOriginPartitionsAndVersion() throws Exception { + Map pinnedPartsT1 = Maps.newHashMap(); + pinnedPartsT1.put("id=1", listItem(1)); + ConnectorMvccSnapshot connectorSnapshotT1 = ConnectorMvccSnapshot.builder() + .snapshotId(42L).schemaId(7L).build(); + PluginDrivenMvccSnapshot pinT1 = new PluginDrivenMvccSnapshot( + connectorSnapshotT1, pinnedPartsT1, Maps.newHashMap()); + + // A DIFFERENT pin for the SAME table at a second @tag reference. With two non-default versions + // pinned and no default ("") entry, the version-BLIND lookup (StatementContext#getSnapshot(TableIf)) + // is ambiguous and gives up (see its javadoc); only the version-AWARE lookup that the + // LogicalFileScan branch of pinnedSnapshot uses resolves the exact t1 reference. MUTATION: + // collapsing pinnedSnapshot to the version-blind fallback makes this test observably diverge + // (an unresolved pin sends getOrMaterialize to materializeLatest() on a field-less mock, or the + // assertions below simply see the wrong values). + ConnectorMvccSnapshot connectorSnapshotT2 = ConnectorMvccSnapshot.builder() + .snapshotId(99L).schemaId(3L).build(); + PluginDrivenMvccSnapshot pinT2 = new PluginDrivenMvccSnapshot( + connectorSnapshotT2, Maps.newHashMap(), Maps.newHashMap()); + + // NOTE: table's default answer is CALLS_REAL_METHODS, so every stub below MUST use the + // doReturn(...).when(table)... form (never when(table.foo()).thenReturn(...)) -- the latter would + // evaluate table.foo() for REAL as part of recording the stub, exactly the pitfall Mockito spies + // have, and several of these real bodies dereference fields this field-less mock never set. + PluginDrivenMvccExternalTable table = + Mockito.mock(PluginDrivenMvccExternalTable.class, Mockito.CALLS_REAL_METHODS); + ExternalDatabase database = Mockito.mock(ExternalDatabase.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.doReturn(TBL).when(table).getName(); + Mockito.doReturn(database).when(table).getDatabase(); + Mockito.when(database.getFullName()).thenReturn(DB); + Mockito.when(database.getCatalog()).thenReturn((CatalogIf) catalog); + Mockito.when(catalog.getName()).thenReturn(CTL); + // Not under test here (see LogicalFileScanTest precedent): bypass its real body, which needs + // schema/catalog wiring this bare mock doesn't carry. + Mockito.doReturn(SelectedPartitions.NOT_PRUNED).when(table).initSelectedPartitions(Mockito.any()); + + TableScanParams tagT1 = new TableScanParams("tag", ImmutableMap.of(), ImmutableList.of("t1")); + TableScanParams tagT2 = new TableScanParams("tag", ImmutableMap.of(), ImmutableList.of("t2")); + Mockito.doReturn(pinT1).when(table).loadSnapshot(Optional.empty(), Optional.of(tagT1)); + Mockito.doReturn(pinT2).when(table).loadSnapshot(Optional.empty(), Optional.of(tagT2)); + + ConnectContext ctx = new ConnectContext(); + StatementContext stmtCtx = new StatementContext(ctx, null); + ctx.setStatementContext(stmtCtx); + ctx.setThreadLocalInfo(); + try { + // Pin via loadSnapshots (not a hand-rolled key) so the version key is computed by the SAME + // function the lookup uses -- the test must not hand-roll a key and accidentally agree with + // itself. + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.of(tagT1)); + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.of(tagT2)); + + LogicalFileScan scan = new LogicalFileScan(new RelationId(1), table, + Collections.singletonList(DB), Collections.emptyList(), + Optional.empty(), Optional.empty(), Optional.of(tagT1), Optional.empty()); + + Map origin = table.getOriginPartitions(scan); + Object version = table.getPartitionMetaVersion(scan); + + Assertions.assertEquals(pinnedPartsT1, origin, + "getOriginPartitions must dispatch through the LogicalFileScan branch of pinnedSnapshot " + + "and return exactly the t1 pin's partition map (not t2's, not latest's)"); + Assertions.assertEquals(new java.util.HashSet<>(pinnedPartsT1.keySet()), version, + "getPartitionMetaVersion must return the FROZEN partition NAME-SET of the t1 pin " + + "(a real snapshotId=42 no longer yields the @ token) -- " + + "resolved via the same LogicalFileScan branch, so it is t1's name set, not t2's " + + "(empty) or latest's"); + } finally { + ConnectContext.remove(); + } + } + + // ──────────────────── Cache B version is the frozen partition NAME-SET for ALL engines ───── + // + // getPartitionMetaVersion always returns the frozen partition NAME SET + // (getOriginPartitions(scan).keySet(), copied) -- the SAME map the ranges are built from, so + // version == exact content and Cache B rebuilds precisely when the partition set changes, never on a + // stale set. This is uniform across hive (snapshotId == -1 sentinel), paimon and iceberg: the old + // "@" O(1) token was removed because it is unsafe wherever a connector's + // listPartitions content is not a pure function of the snapshot id (iceberg non-RANGE: identity / + // bucket / truncate / multi-field partitioning), whose nameToPartitionItem (Cache A) and snapshot-id + // token (a DIFFERENT cache) expire independently. PluginDrivenMvccExternalTable no longer overrides + // getSortedPartitionRanges (the -1 short-circuit added in 5c17b748880 was removed): every pin goes + // through the inherited ExternalTable#getSortedPartitionRanges -> the cache manager. + + /** + * Builds a live Env whose {@code getSortedPartitionsCacheManager()} returns {@code rangesCacheMgr}, + * mockStatic-scoped so {@code table.getSortedPartitionRanges(scan)} (now purely inherited from + * {@link ExternalTable}, no override) can resolve {@code Env.getCurrentEnv()} on the real dispatch path. + */ + private static MockedStatic mockEnvWithRangesCacheManager(NereidsSortedPartitionsCacheManager rangesCacheMgr) { + Env env = Mockito.mock(Env.class); + Mockito.when(env.getSortedPartitionsCacheManager()).thenReturn(rangesCacheMgr); + MockedStatic envStatic = Mockito.mockStatic(Env.class); + envStatic.when(Env::getCurrentEnv).thenReturn(env); + return envStatic; + } + + /** Builds a {@code PluginDrivenMvccExternalTable} mock wired for the real getOriginPartitions/getPartitionMetaVersion/pinnedSnapshot bodies (same technique as the other real-wiring tests in this file). */ + private static PluginDrivenMvccExternalTable newRealWiringTable() { + PluginDrivenMvccExternalTable table = + Mockito.mock(PluginDrivenMvccExternalTable.class, Mockito.CALLS_REAL_METHODS); + ExternalDatabase database = Mockito.mock(ExternalDatabase.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.doReturn(TBL).when(table).getName(); + Mockito.doReturn(database).when(table).getDatabase(); + Mockito.when(database.getFullName()).thenReturn(DB); + Mockito.when(database.getCatalog()).thenReturn((CatalogIf) catalog); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.doReturn(SelectedPartitions.NOT_PRUNED).when(table).initSelectedPartitions(Mockito.any()); + return table; + } + + /** Pins {@code pin} onto a FRESH ConnectContext/StatementContext ("a new query") and returns the scan built over it. */ + private static LogicalFileScan pinLatestAndBuildScan(PluginDrivenMvccExternalTable table, PluginDrivenMvccSnapshot pin) { + ConnectContext ctx = new ConnectContext(); + StatementContext stmtCtx = new StatementContext(ctx, null); + ctx.setStatementContext(stmtCtx); + ctx.setThreadLocalInfo(); + // B5a implicit query-begin (latest) pin -- mirrors a plain (no @tag/@branch/time-travel) hive scan. + Mockito.doReturn(pin).when(table).loadSnapshot(Optional.empty(), Optional.empty()); + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.empty()); + return new LogicalFileScan(new RelationId(1), table, + Collections.singletonList(DB), Collections.emptyList(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + } + + private static PluginDrivenMvccSnapshot sentinelPin(Map parts) { + ConnectorMvccSnapshot sentinelSnapshot = ConnectorMvccSnapshot.builder() + .snapshotId(-1L).schemaId(0L).build(); + return new PluginDrivenMvccSnapshot(sentinelSnapshot, parts, Maps.newHashMap()); + } + + /** + * A pin carrying a REAL (non-sentinel) connector snapshot id -- e.g. iceberg. Used to prove Cache B's + * version token is the frozen partition NAME-SET even for {@code snapshotId != -1}: the snapshot id is + * held CONSTANT across queries while the partition set changes, exactly the iceberg non-RANGE hazard + * where Cache A (listPartitions) and the snapshot-id token expire independently. + */ + private static PluginDrivenMvccSnapshot realSnapshotPin(Map parts) { + ConnectorMvccSnapshot realSnapshot = ConnectorMvccSnapshot.builder() + .snapshotId(42L).schemaId(7L).build(); + return new PluginDrivenMvccSnapshot(realSnapshot, parts, Maps.newHashMap()); + } + + @Test + public void testGetSortedPartitionRangesPresentForSnapshotLessNonEmptyPartitions() throws Exception { + Map pinnedParts = Maps.newHashMap(); + pinnedParts.put("id=1", listItem(1)); + pinnedParts.put("id=2", listItem(2)); + PluginDrivenMvccExternalTable table = newRealWiringTable(); + + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + LogicalFileScan scan = pinLatestAndBuildScan(table, sentinelPin(pinnedParts)); + + Optional> ranges = table.getSortedPartitionRanges(scan); + + Assertions.assertTrue(ranges.isPresent(), + "snapshotId == -1 (hive sentinel) with a non-empty partition set must now use Cache B, " + + "keyed by the partition NAME-SET version token"); + } finally { + ConnectContext.remove(); + } + } + + @Test + public void testGetSortedPartitionRangesRebuildsWhenSnapshotLessNameSetChanges() throws Exception { + PluginDrivenMvccExternalTable table = newRealWiringTable(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + Map partsAtQuery1 = Maps.newHashMap(); + partsAtQuery1.put("id=1", listItem(1)); + partsAtQuery1.put("id=2", listItem(2)); + LogicalFileScan scan1 = pinLatestAndBuildScan(table, sentinelPin(partsAtQuery1)); + SortedPartitionRanges first = table.getSortedPartitionRanges(scan1).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at the first (2-partition) name set"); + Assertions.assertEquals(2, first.sortedPartitions.size()); + + // A second query ("ALTER ADD PARTITION" between queries): the pin's partition NAME SET grew. + // The manager's Objects.equals compares the two HashSet version tokens by CONTENT, so it must + // detect this change even though snapshotId is still the constant -1 on both pins. + Map partsAtQuery2 = Maps.newHashMap(); + partsAtQuery2.put("id=1", listItem(1)); + partsAtQuery2.put("id=2", listItem(2)); + partsAtQuery2.put("id=3", listItem(3)); + LogicalFileScan scan2 = pinLatestAndBuildScan(table, sentinelPin(partsAtQuery2)); + SortedPartitionRanges rebuilt = table.getSortedPartitionRanges(scan2).orElse(null); + + Assertions.assertNotNull(rebuilt, "ranges rebuilt at the second (3-partition) name set"); + Assertions.assertNotSame(first, rebuilt, + "name-set change at a constant snapshotId==-1 must still trigger a rebuild " + + "(the version token is content-derived from the partition names, not the snapshot id)"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + } finally { + ConnectContext.remove(); + } + } + + @Test + public void testGetSortedPartitionRangesRebuildsWhenRealSnapshotNameSetChanges() throws Exception { + // Real-snapshot (snapshotId != -1) coherence: the version token is the frozen partition NAME-SET + // for ALL engines, not just hive. This is the iceberg non-RANGE hazard -- the pin's + // nameToPartitionItem (served by listPartitions / Cache A) can advance while the connector snapshot + // id stays CONSTANT, because the two are backed by INDEPENDENTLY-expiring caches. Under the removed + // "@" token both queries would key "42@7" and serve a STALE HIT (silent + // under-inclusive pruning); under the name-set token the grown partition set forces a rebuild. + PluginDrivenMvccExternalTable table = newRealWiringTable(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + Map partsAtQuery1 = Maps.newHashMap(); + partsAtQuery1.put("id=1", listItem(1)); + partsAtQuery1.put("id=2", listItem(2)); + LogicalFileScan scan1 = pinLatestAndBuildScan(table, realSnapshotPin(partsAtQuery1)); + SortedPartitionRanges first = table.getSortedPartitionRanges(scan1).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at the first (2-partition) name set"); + Assertions.assertEquals(2, first.sortedPartitions.size()); + + // Same query again: IDENTICAL name set at the SAME real snapshot id => cache HIT (same instance). + LogicalFileScan scanHit = pinLatestAndBuildScan(table, realSnapshotPin(partsAtQuery1)); + SortedPartitionRanges hit = table.getSortedPartitionRanges(scanHit).orElse(null); + Assertions.assertSame(first, hit, + "unchanged name set at a real snapshotId=42 => cache HIT returns the SAME instance"); + + // A later query whose Cache A refreshed to include a new partition while the connector snapshot + // id is STILL 42: the name set grew, so Cache B must rebuild even though "42@7" is unchanged. + Map partsAtQuery2 = Maps.newHashMap(); + partsAtQuery2.put("id=1", listItem(1)); + partsAtQuery2.put("id=2", listItem(2)); + partsAtQuery2.put("id=3", listItem(3)); + LogicalFileScan scan2 = pinLatestAndBuildScan(table, realSnapshotPin(partsAtQuery2)); + SortedPartitionRanges rebuilt = table.getSortedPartitionRanges(scan2).orElse(null); + Assertions.assertNotNull(rebuilt, "ranges rebuilt at the second (3-partition) name set"); + Assertions.assertNotSame(first, rebuilt, + "name-set change at a CONSTANT real snapshotId=42 must still trigger a rebuild " + + "(the removed @ token would have served a stale hit)"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + } finally { + ConnectContext.remove(); + } + } + + @Test + public void testGetSortedPartitionRangesEmptyForSnapshotLessEmptyPartitions() throws Exception { + PluginDrivenMvccExternalTable table = newRealWiringTable(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + LogicalFileScan scan = pinLatestAndBuildScan(table, sentinelPin(Maps.newHashMap())); + + Optional> ranges = table.getSortedPartitionRanges(scan); + + Assertions.assertFalse(ranges.isPresent(), + "an empty partition set (snapshot-less or not) yields no ranges to cache " + + "(SortedPartitionRanges.build(emptyMap) == null)"); + } finally { + ConnectContext.remove(); + } + } + + // ──────────────────── Real production wiring: ExternalMetaCacheMgr.invalidateTable ──────────────────── + + @Test + public void testExternalMetaCacheMgrInvalidateTableDropsRangesCacheEntry() throws Exception { + // Drives the REAL ExternalMetaCacheMgr.invalidateTable(...) (the new call at + // ExternalMetaCacheMgr.java:217-220), not NereidsSortedPartitionsCacheManager directly, so the + // production wiring between the two managers is exercised end-to-end. + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + Assertions.assertTrue(rangesCacheMgr.get(t.table, (CatalogRelation) null).isPresent(), + "ranges built and cached before invalidation"); + Assertions.assertEquals(1, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "one entry cached before invalidation"); + + long catalogId = 7L; + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.when(catalogMgr.getCatalog(catalogId)).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(env.getSortedPartitionsCacheManager()).thenReturn(rangesCacheMgr); + + ExternalMetaCacheMgr metaCacheMgr = new ExternalMetaCacheMgr(true); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + metaCacheMgr.invalidateTable(catalogId, DB, TBL); + } + + Assertions.assertEquals(0, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "ExternalMetaCacheMgr.invalidateTable must also drop the " + + "NereidsSortedPartitionsCacheManager entry (ExternalMetaCacheMgr.java:217-220)"); + } + + // ── db/catalog-level invalidation also drops Cache B (§10 completeness) ── + // + // invalidateTable (above) already drops the ranges cache; invalidateDb / invalidateCatalog / + // removeCatalog previously did NOT, so a db- or catalog-level REFRESH left STALE Cache B entries. + // Cache B has no db/catalog-scoped eviction key, so those coarse invalidations drop ALL entries + // (invalidateAll). Each test drives the REAL ExternalMetaCacheMgr method end-to-end. + + /** + * Populates a live {@link NereidsSortedPartitionsCacheManager} with one entry, then runs {@code + * invalidation} against a REAL {@link ExternalMetaCacheMgr} (with a mocked {@code Env} wiring + * {@code getCatalogMgr}/{@code getSortedPartitionsCacheManager}) and asserts Cache B is emptied. + */ + private void assertDropsAllRangesCache(java.util.function.Consumer invalidation) + throws Exception { + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + Assertions.assertTrue(rangesCacheMgr.get(t.table, (CatalogRelation) null).isPresent(), + "ranges built and cached before invalidation"); + Assertions.assertEquals(1, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "one entry cached before invalidation"); + + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.when(catalogMgr.getCatalog(CATALOG_ID)).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(env.getSortedPartitionsCacheManager()).thenReturn(rangesCacheMgr); + + ExternalMetaCacheMgr metaCacheMgr = new ExternalMetaCacheMgr(true); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + invalidation.accept(metaCacheMgr); + } + + Assertions.assertEquals(0, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "db/catalog-level invalidation must also drop the NereidsSortedPartitionsCacheManager entries"); + } + + @Test + public void testInvalidateDbDropsRangesCache() throws Exception { + assertDropsAllRangesCache(m -> m.invalidateDb(CATALOG_ID, DB)); + } + + @Test + public void testInvalidateCatalogDropsRangesCache() throws Exception { + assertDropsAllRangesCache(m -> m.invalidateCatalog(CATALOG_ID)); + } + + @Test + public void testRemoveCatalogDropsRangesCache() throws Exception { + assertDropsAllRangesCache(m -> m.removeCatalog(CATALOG_ID)); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java index 9aa92915a96e13..68ed38f3df2b07 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java @@ -23,6 +23,7 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorMetadata; import org.apache.doris.connector.api.ConnectorSession; +import org.apache.doris.connector.api.ConnectorStatementScope; import org.apache.doris.connector.api.handle.ConnectorTableHandle; import org.apache.doris.connector.api.write.ConnectorWritePartitionField; import org.apache.doris.connector.api.write.ConnectorWritePartitionSpec; @@ -313,6 +314,9 @@ private static PluginDrivenExternalTable pluginTable(ConnectorWritePartitionSpec ConnectorTableHandle handle = Mockito.mock(ConnectorTableHandle.class); ConnectorMetadata metadata = Mockito.mock(ConnectorMetadata.class); ConnectorSession session = Mockito.mock(ConnectorSession.class); + // The sink resolves metadata through PluginDrivenMetadata.get, which memoizes on the session's + // per-statement scope; NONE runs the loader every call (byte-identical to a direct getMetadata). + Mockito.when(session.getStatementScope()).thenReturn(ConnectorStatementScope.NONE); Connector connector = Mockito.mock(Connector.class); Mockito.when(connector.getWritePlanProvider()).thenReturn(provider); // Production selects the write provider per-handle; a plain mock does not run the interface default. diff --git a/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md b/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md new file mode 100644 index 00000000000000..274e5d51785257 --- /dev/null +++ b/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md @@ -0,0 +1,179 @@ +# 外表分区"派生视图"二级缓存设计（连接器视图缓存 + 接入 NereidsSortedPartitionsCacheManager） + +- 日期：2026-07-20 +- 分支：`branch-catalog-spi` +- 状态：设计已评审通过，待实现计划 +- 关联：CACHE-P1（翻闸弃二级缓存决策）、#65659（分区裁剪 TOCTOU freeze） + +## 1. 问题背景 + +catalog-SPI 翻闸（CACHE-P1 决策）把 legacy fe-core 里引擎特定的"分区派生视图"二级缓存整层删除，改为**每查询直连列举分区**。代价（已登记为 CACHE-P1 偏差、known-degradation）分两块，按引擎轻重不同： + +1. **远程枚举成本**：iceberg 分区表每查询一次 `PARTITIONS` 元数据表扫描（loadTable + 遍历 manifest-list + 全 manifest + avro 解码）；maxcompute 每查询一次 ODPS `getPartitions()`；hive 每查询把 HMS 名单解析成 `ConnectorPartitionInfo`（原始名单已被 `CachingHmsClient` 缓存，此处是解析 CPU）。 +2. **派生视图重建成本**：fe-core 每查询把分区名单构建成 `nameToPartitionItem`（`PartitionItem`），并在 `PruneFileScanPartition` 里用 `SortedPartitionRanges.build()` 重建二分查找结构（O(n) 建 range + O(n log n) 排序）。legacy 里这份 `SortedPartitionRanges` 是缓存复用的（`HiveExternalMetaCache.HivePartitionValues` 构造函数里建一次、跨查询复用），翻闸后变成每查询重建。 + +对**大量分区**表，这两块都显著。本设计恢复这两层缓存，同时满足 SPI 隔离、且不复发 #65659 的 TOCTOU。 + +## 2. 目标 / 非目标 + +**目标** +- 恢复跨查询的分区派生视图缓存，消除上述两块重复成本。 +- 严格满足 SPI 隔离：fe-core 保持连接器无关；连接器侧缓存建在 SPI 线以下。 +- 不复发 #65659 的分区视图查询内分叉（TOCTOU）。 +- 通用框架，全 SPI 引擎（`SPI_READY_TYPES = jdbc, es, trino-connector, max_compute, paimon, iceberg, hms`）可受益；按收益接线（iceberg/paimon/hive/maxcompute 先接，jdbc/es 分区退化可暂 no-op）。 + +**非目标** +- 不引入全局 as-of 快照语义；跨查询采用 bounded-staleness 契约（TTL + 失效钩子）。 +- 不改变 `PruneFileScanPartition` 的二分裁剪算法（`PartitionPruner.binarySearchFiltering` 保持原样）。 +- 不为带谓词的 `listPartitions`（当前不存在此调用）设计缓存分桶。 +- **不考虑 `use_meta_cache=false`**：新 SPI 框架下 `use_meta_cache` **恒为 true**（连接器始终经元数据缓存），无直连绕过路径需设计。SHOW PARTITIONS / partitions-TVF 的"取新"仍由既有 per-call fresh 路径（`listPartitionNamesFresh` 等）承担，与本缓存正交、绕过缓存 A。 + +## 3. 现状梳理（设计所依赖的事实） + +- **每语句冻结层已存在（相当于 Trino 的 per-transaction memoizer）**：hive/iceberg/paimon 都是 `PluginDrivenMvccExternalTable`；分区视图在 `materializeLatest()`（`PluginDrivenMvccExternalTable.java:127`）**每语句只物化一次**，冻结进 `PluginDrivenMvccSnapshot`（存于 `StatementContext.snapshots`，per-statement 实例字段）。`PruneFileScanPartition` 读的是冻结的 `SelectedPartitions`，绝不回连缓存——这是 #65659 TOCTOU 不复发的结构性原因。 +- **裁剪路径 `listPartitions` 永远不带 filter**：全部 4 个调用点（`PluginDrivenExternalTable.java:831/:887`、`PluginDrivenMvccExternalTable.java:271`、`ShowPartitionsCommand.java:300`）传 `Optional.empty()`。 +- **二分裁剪机制在分支中完好且已在用**：`PartitionPruner.pruneWithResult`（`PartitionPruner.java:163`）在 `sortedPartitionRanges.isPresent()` 时走 `binarySearchFiltering`（`:204-209, :255`），由 session 变量 `enableBinarySearchFilteringPartitions` 控制；#65659 合并后 `PruneFileScanPartition` 每查询用 `.or(() -> SortedPartitionRanges.build(nameToPartitionItem))`（`PruneFileScanPartition.java:101-102`）现建 ranges 后照常二分——功能与 legacy 一致，唯缺缓存复用。 +- **fe-core 已有引擎无关、按版本 keyed 的 ranges 缓存**：`NereidsSortedPartitionsCacheManager`（`common/cache/NereidsSortedPartitionsCacheManager.java`），key=`TableIdentifier(catalog,db,table)`，`PartitionCacheContext(tableId, partitionMetaVersion, sortedPartitionRanges)`，`get()`（`:74`）用 `getPartitionMetaVersion(scan)` 校验版本、变更即重建（`:98-102`），`loadCache()`（`:117`）带"插入过频跳过排序"启发式（`:127-131`），软值 + FE 配置 TTL。**OLAP 表已通过 `PruneOlapScanPartition.java:161-168` 在用**。其接口 `SupportBinarySearchFilteringPartitions`（`catalog/SupportBinarySearchFilteringPartitions.java`）javadoc 明言"you can save the partition's meta snapshot id in the CatalogRelation and get the partitions by the snapshot id"——为外表 MVCC 快照场景预留。 +- **外表当前完全绕过它**：`ExternalTable.getSortedPartitionRanges(scan)`（`ExternalTable.java:527`）返回 `Optional.empty()` 空桩，且 #65659 合并后**已无任何调用者**（死代码）。 +- **已有连接器侧缓存基建**：`fe-connector-cache` 的 `CacheSpec`（属性模型 `meta.cache...(enable|ttl-second|capacity)`）+ `MetaCacheEntry`（Caffeine，contextual-only + manual-miss，generation 计数失效）。iceberg 已有 `IcebergPartitionCache`（key=`(TableIdentifier, snapshotId)` → `List` **原始行**）、`IcebergLatestSnapshotCache`（`(snapshotId, schemaId)` 快照钉子）；hive 有 `CachingHmsClient`（缓存 `listPartitionNames`/`getPartitions` 原始 HMS 元数据）。**但派生视图（`ConnectorMvccPartitionView` / `List`）每次重算**。 +- **连接器失效缝**：`Connector.invalidateTable/invalidateDb/invalidateAll/invalidatePartition`（`fe-connector-api/.../Connector.java:312/316/324/336`，默认 no-op，引擎 override）。REFRESH TABLE（`RefreshManager.refreshTableInternal` → `connector.invalidateTable`）/CATALOG（`PluginDrivenExternalCatalog.onRefreshCache` → `connector.invalidateAll`）/DB + HMS 事件（ADD/DROP/REFRESH_PARTITIONS → `connector.invalidatePartition`）已全部 fan-out 到这 4 个钩子。 +- **session=user 凭证隔离**：iceberg 连接器缓存在 `iceberg.rest.session=user` 下被置 `null` 禁用（`check-authz-cache-sharding.sh` 标记守护）。 + +## 4. 方案总览：两层互补缓存 + +``` +Nereids 裁剪 (PruneFileScanPartition.pruneExternalPartitions) + │ nameToPartitionItem = scan.getSelectedPartitions().selectedPartitions // 冻结 (T1) + │ ranges = scan.getSelectedPartitions().sortedPartitionRanges // 冻结 (#65659), 分支恒空 + │ .or(() -> externalTable.getSortedPartitionRanges(scan)) // ← 复活死桩 → 缓存 B + │ .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))) // 最后兜底 + │ → PartitionPruner.pruneWithResult(..., ranges) → binarySearchFiltering // 机制不变 + ▼ +[缓存 B · fe-core 已存在] NereidsSortedPartitionsCacheManager + key = (catalog, db, table) + validity = getPartitionMetaVersion(scan) // = pinned metaVersion, 见 §6 + value = 预建 SortedPartitionRanges + origin = getOriginPartitions(scan) → pinned 快照的冻结 nameToPartitionItem + ── 消除: 每查询重建 SortedPartitionRanges (大量分区 CPU) + ▲ +[Tier-B 冻结 · 已存在] PluginDrivenMvccSnapshot (每语句物化一次, materializeLatest) + ▲ ── 唯一读下层缓存处 + │ +[缓存 A · 新增, SPI 线以下] ConnectorPartitionViewCache (各连接器 final 字段) + key = (db, table, snapshotId, schemaId) + value = List / ConnectorMvccPartitionView + ── 消除: 远程枚举 (iceberg PARTITIONS 扫描 / ODPS getPartitions / hive 名单解析) + ▲ +[remote] iceberg catalog / hive HMS / odps +``` + +对照 Trino：缓存 A = Trino 的"共享 TTL 层"（`SharedHiveMetastoreCache`，跨查询、可陈旧、SPI 线以下）；Tier-B 冻结层 = Trino 的"per-transaction memoizer"（首次触碰即冻结、查询内一致）。区别是 Doris 的 per-statement 冻结层已存在，故只需补共享层 A + 复用已有 ranges 缓存 B。 + +## 5. 缓存 A：`ConnectorPartitionViewCache`（SPI 线以下） + +**放置**：`fe/fe-connector/fe-connector-cache/.../cache/ConnectorPartitionViewCache.java`（共享工具，非 SPI 接口；与 `CachingHmsClient`/`IcebergPartitionCache` 同一构建方式）。各连接器持有一个 final 字段实例。 + +**形态**：内部一个 `MetaCacheEntry`，沿用 contextual-only + manual-miss 并发模型。 +- key `PartitionViewCacheKey`：`(db, table, snapshotId, schemaId)`。**不含 filter**（裁剪路径恒空；带 filter 的 `listPartitions` 由 loader 一行 `if (filter.isPresent()) return uncachedLoad();` 绕过）。 +- value：`List`（iceberg 另可缓存派生的 `ConnectorMvccPartitionView`）。 +- 配置：`CacheSpec` 键 `meta.cache..partition_view.(enable|ttl-second|capacity)`，默认 TTL 86400s、ON、capacity 与同引擎其它 entry 一致。 + +**接入点**（把原本昂贵的枚举+派生 body 包进 `cache.get(key, () -> body)`）： +- iceberg：`IcebergConnectorMetadata.getMvccPartitionView` / `listPartitions`，叠在现有 `IcebergPartitionCache`（raw 行）之上，缓存派生视图。 +- paimon：`getMvccPartitionView` / `listPartitions`，`(snapshotId, schemaId)` keyed。 +- hive：`HiveConnectorMetadata.listPartitions`，`(db, table)` keyed（snapshotId=-1）+ TTL/失效。 +- maxcompute：`listPartitions`，`(db, table)` keyed + TTL/失效。 +- jdbc/es/trino：分区退化/罕见，框架可接，本期 no-op。 + +**失效**：在各连接器已 override 的 `invalidateTable/invalidateDb/invalidateAll/invalidatePartition` 里加 `partitionViewCache.invalidate*`。`invalidatePartition`（SPI 只带 values 不带 name）对本缓存按 `(db,table)` 整表失效（本就是整表视图，粒度天然对齐、偏保守=安全）。iceberg 因 key 带 snapshotId，REFRESH 后新 snapshot 自然 miss，失效为冗余保险。 + +**session=user**：沿用现有规则——`iceberg.rest.session=user` 下该连接器缓存置 `null` 禁用，挂 `check-authz-cache-sharding.sh` 标记。 + +## 6. 缓存 B：接入 `NereidsSortedPartitionsCacheManager` + +**让外表实现 `SupportBinarySearchFilteringPartitions`**（`PluginDrivenMvccExternalTable` 覆盖 hive/iceberg/paimon；`PluginDrivenExternalTable` 覆盖 maxcompute 等非 MVCC）： +- `getOriginPartitions(scan)`：返回该 scan pinned 快照的**冻结 `nameToPartitionItem`**（`PluginDrivenMvccSnapshot.getNameToPartitionItem()`，数据来自缓存 A / 冻结层，非重新远程列举）。 +- `getPartitionMetaVersion(scan)`：版本令牌（见下）。 +- `getPartitionMetaLoadTimeMillis(scan)`：该分区元数据物化时刻（喂"插入过频跳过排序"启发式；取快照 pin / 缓存 A 条目载入时刻）。 + +**复活死桩**：`ExternalTable.getSortedPartitionRanges(scan)` 改为委托 `Env.getCurrentEnv().getSortedPartitionsCacheManager().get(this, scan)`——与 OLAP 的 `PruneOlapScanPartition` 同一套。 + +**裁剪路径接线**：`PruneFileScanPartition.pruneExternalPartitions` 的 ranges 获取改为： +```java +sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges // 冻结优先 (#65659) + .or(() -> (Optional) externalTable.getSortedPartitionRanges(scan)) // 缓存 B + .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))); // 兜底 +``` +保留 #65659"冻结字段优先"的语义；缓存 B 命中即复用预建 ranges；缓存 B 返回空（跳过排序启发式/禁用）时退回现建；二者皆从 pinned 快照的同一 map 派生。 + +## 7. 版本令牌（缓存 B 正确性与两层联动） + +统一走**已每语句穿到底的 SPI 类型 `ConnectorMvccSnapshot` 上新增的一个 `metaVersion` 字段**（不新增 SPI 方法，只在既有类型加字段）： +- **iceberg / paimon**：`metaVersion = (snapshotId, schemaId)`——不可变、精确（`ConnectorMvccSnapshot.getSnapshotId():55 / getSchemaId():73` 已有；iceberg 由 `loadLatestSnapshotPin`（`IcebergConnectorMetadata.java:1630`）原子捕获）。新快照自然换 key。 +- **hive / 非 MVCC**：`metaVersion = 连接器维护的单调 generation`（连接器内 `ConcurrentHashMap`，`invalidateTable/invalidatePartition` 时 bump、缓存 A reload 时 bump）。廉价 in-memory、精确，且**让缓存 A 与缓存 B 版本联动**（B 的 ranges 绝不比 A 的视图更旧）。 + +`getPartitionMetaVersion(scan)` 从 scan 的 pinned 快照读出该 `metaVersion`（scan 携带 `tableSnapshot`/`scanParams` → `StatementContext.getSnapshot` → pinned `PluginDrivenMvccSnapshot` → `ConnectorMvccSnapshot.metaVersion`）。 + +## 8. 正确性论证（两层皆不复发 #65659 TOCTOU） + +- **缓存 A**：只在 `materializeLatest()` 那一次被读，读出即冻结进 `PluginDrivenMvccSnapshot`；`PruneFileScanPartition` 只读冻结副本，绝不回连缓存 A。同一 key 并发命中返回同一不可变 value。跨查询：MVCC 令牌保证永不把 S+1 当 S；hive 为 bounded-staleness（TTL + 失效钩子，= `use_meta_cache` 契约）。 +- **缓存 B**：`getPartitionMetaVersion(scan)` = pinned 快照/generation，`getOriginPartitions(scan)` 返回同一 pinned 快照的冻结 `nameToPartitionItem`——ranges 与 `nameToPartitionItem` **同源**；版本不符即重建。故 `prunedPartitions ⊆ nameToPartitionItem.keySet()` 恒成立，`PruneFileScanPartition` 的 `Preconditions.checkState(item != null, ...)`（#65659）永不触发。 +- **查询内一致**由既有冻结层保证（物化一次）；**跨查询**由版本令牌（MVCC）/ TTL+失效（hive）保证 bounded staleness。二者组合等价于 Trino 的"共享 TTL 层 + per-transaction memoizer"。 + +## 9. 隔离与配置 + +- **SPI 隔离**：缓存 A 完全在 SPI 线以下（连接器 final 字段，缓存 SPI 类型），fe-core 一行不改。缓存 B 在 fe-core，但 key=`(表)+不透明 SPI 令牌`、`getOriginPartitions` 只消费 SPI 输出、失效走连接器钩子——不碰引擎内部，与 OLAP 同构。 +- **session=user**：缓存 A 沿用"置 null 禁用"。缓存 B 只存 `(表,快照)` 维度 ranges、**不含任何凭证**，与 OLAP 同样跨用户共享安全（同 snapshotId ⇒ 同分区集），不禁用。 +- **配置开关**：缓存 A `meta.cache..partition_view.(enable|ttl-second|capacity)`（默认 ON/86400s），`enable=false`/`ttl=0` 一键关（对应 Trino `cache-partitions=false` 逃生阀）；缓存 B 复用现有 session 变量 `enableBinarySearchFilteringPartitions` + `cacheSortedPartitionIntervalSecond` + FE 配置（`NereidsSortedPartitionsCacheManager` 现有的 `sortedPartitionTableManageNum`/`expireSortedPartitionTableInFeSecond`）。 + +## 10. 失效汇总（复用现成管线，零新增管线） + +| 触发 | 现有 fan-out | 本设计追加 | +|---|---|---| +| REFRESH TABLE / HMS REFRESH_TABLE | `RefreshManager.refreshTableInternal` → `connector.invalidateTable` | 钩子内 `partitionViewCache.invalidateTable` + bump generation；`NereidsSortedPartitionsCacheManager.invalidate(catalog,db,table)` | +| REFRESH CATALOG | `PluginDrivenExternalCatalog.onRefreshCache` → `connector.invalidateAll` | `partitionViewCache.invalidateAll`；ranges 缓存对应库表失效 | +| REFRESH DB | `connector.invalidateDb` | `partitionViewCache.invalidateDb` | +| HMS ADD/DROP/REFRESH_PARTITIONS | `connector.invalidatePartition` | `partitionViewCache.invalidateTable`(整表) + bump generation → 缓存 B 版本自然失配重建 | +| DROP/RENAME TABLE, DROP DB | `connector.invalidateTable/invalidateDb` | 同上 | + +## 11. 测试 + +**单元测试（我方跑）** +- 缓存 A：per-engine miss→hit；按 `(snapshotId,schemaId)` 分桶（不同快照互不串味）；4 钩子失效各自生效；`session=user` 下禁用（返回 null 走直连）；带 filter 绕过。仿 `IcebergPartitionCacheTest`/`CachingHmsClientTest`。 +- 缓存 B：外表实现 `SupportBinarySearchFilteringPartitions` 三方法；版本变更（snapshotId 变 / generation bump）触发重建；死桩 `getSortedPartitionRanges` 委托生效；`getOriginPartitions` 与冻结 map 同源 → `prunedPartitions ⊆ keySet`，`Preconditions` 不触发；"插入过频跳过排序"启发式。 +- `PruneFileScanPartition`：二分裁剪结果与 legacy/兜底 `build()` 一致的回归（同一谓词、同一分区集 → 同一 pruned 集）。 +- 复用/借鉴 `BinarySearchPartitionInconsistencyTest`（#65659）验证冻结一致性不被破坏。 + +**e2e（用户跑）** +- 大量分区外表重复查询：第二次起命中缓存 A（无远程枚举）、命中缓存 B（无 ranges 重建）。 +- `ALTER TABLE ADD/DROP PARTITION` 后失效可见（下次查询反映新分区集）。 +- iceberg 时间旅行 `FOR VERSION/TIME AS OF` 不串味（不同 snapshotId 独立缓存）。 +- `set enableBinarySearchFilteringPartitions=false` / `meta.cache..partition_view.enable=false` 回退行为。 +- `REFRESH TABLE`/`REFRESH CATALOG` 立即生效。 + +## 12. 风险与回滚 + +- 缓存 A 纯旁路：`enable=false` 即回到当前每查询枚举行为。 +- 缓存 B 是复用既有 OLAP 机制 + 复活死桩：`enableBinarySearchFilteringPartitions=false` 即回到 #65659 的每查询 `build()` 行为。 +- 均不改冻结层、不动 `PartitionPruner`/`PruneFileScanPartition` 的裁剪算法，故不影响 #65659 的正确性结论。 +- 主要风险面 = 失效遗漏导致 bounded staleness 超预期；缓解 = 两层失效均挂同一批已验证的连接器钩子 + TTL 上限。 + +## 13. 影响文件清单（预估） + +**新增** +- `fe/fe-connector/fe-connector-cache/.../cache/ConnectorPartitionViewCache.java`（+ 测试） +- `fe/fe-connector/fe-connector-cache/.../cache/PartitionViewCacheKey.java` + +**修改** +- SPI：`fe-connector-*/.../ConnectorMvccSnapshot`（加 `metaVersion` 字段 + builder）。 +- iceberg：`IcebergConnector`（持有缓存 A + session=user 置 null + 4 钩子）、`IcebergConnectorMetadata`（`getMvccPartitionView`/`listPartitions` 包缓存、`beginQuerySnapshot` 填 `metaVersion`）。 +- paimon：`PaimonConnector`/`PaimonConnectorMetadata` 同构。 +- hive：`HiveConnector`（缓存 A + generation map + 钩子）、`HiveConnectorMetadata`（`listPartitions` 包缓存、`beginQuerySnapshot` 填 generation）。 +- maxcompute：`MaxComputeConnector`/`MaxComputeConnectorMetadata` 同 hive 模式。 +- fe-core：`ExternalTable`（`getSortedPartitionRanges` 委托 `NereidsSortedPartitionsCacheManager`）、`PluginDrivenMvccExternalTable`/`PluginDrivenExternalTable`（实现 `SupportBinarySearchFilteringPartitions` 三方法）、`PruneFileScanPartition`（ranges 获取链加缓存 B）。 +- 失效：各连接器 `invalidate*` 钩子内追加两层失效调用。 + +## 14. 遗留 / follow-up（本设计不做） + +- jdbc/es/trino 连接器接入缓存 A（分区退化，收益低，待需求驱动）。 +- 若 profiling 显示某引擎 `getMvccPartitionView` 派生（dedup+buildRange+merge）仍是瓶颈，可在连接器内进一步拆分缓存粒度。 diff --git a/plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md b/plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md new file mode 100644 index 00000000000000..43122849289dd6 --- /dev/null +++ b/plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md @@ -0,0 +1,419 @@ +# Cache B — External SortedPartitionRanges Reuse (iceberg + paimon) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Restore cross-query caching of the pre-built `SortedPartitionRanges` for MVCC external tables (iceberg, paimon) by wiring them into the existing fe-core `NereidsSortedPartitionsCacheManager`, eliminating the per-query rebuild for many-partition pruning — without SPI changes or re-introducing the #65659 TOCTOU. + +**Architecture:** External MVCC tables implement the existing `SupportBinarySearchFilteringPartitions` interface (the same one OLAP tables use), sourcing the version token from the already-pinned `ConnectorMvccSnapshot(snapshotId, schemaId)` and the origin partition map from the frozen per-statement `PluginDrivenMvccSnapshot`. The dead `ExternalTable.getSortedPartitionRanges` stub is revived to delegate to `NereidsSortedPartitionsCacheManager`, and `PruneFileScanPartition` consults it before its build-fresh fallback. Because the cache is keyed by the pinned snapshot and reads the frozen map, ranges stay consistent with `nameToPartitionItem` (no TOCTOU). + +**Tech Stack:** Java 8, Maven (Doris fe reactor), JUnit 5, Caffeine (via existing `NereidsSortedPartitionsCacheManager`). + +## Global Constraints + +- fe-core stays connector-agnostic: no engine-specific (iceberg/paimon) types leak into `ExternalTable`/`PluginDrivenMvccExternalTable` generic methods; the version token is the opaque `(snapshotId, schemaId)` pair from the SPI type `ConnectorMvccSnapshot`. +- Do NOT change the pruning algorithm in `PartitionPruner.binarySearchFiltering`. +- Preserve #65659 semantics: `PruneFileScanPartition` reads the frozen `scan.getSelectedPartitions().sortedPartitionRanges` first; the cache is only a fallback source before `SortedPartitionRanges.build(...)`. +- Gated by the existing session variable `enableBinarySearchFilteringPartitions` (already checked inside `NereidsSortedPartitionsCacheManager.get`). +- `use_meta_cache` is invariantly `true`; no bypass path. +- Scope of THIS plan: iceberg + paimon only (true MVCC with a real `snapshotId`). hive/maxcompute (which need a connector generation token) and Cache A (connector-view cache) are separate plans. +- Build/test invocation (per repo gotchas): run fe-core unit tests with + `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest= -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` + (a full `mvn -f fe/pom.xml install -DskipTests -Dmaven.build.cache.enabled=false` once first, so deps + `${revision}` resolve). + +--- + +## File Structure + +- `fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java` — implement `SupportBinarySearchFilteringPartitions` (3 methods) [Task 1]. +- `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java` — revive `getSortedPartitionRanges` to delegate to the cache manager [Task 2]. +- `fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java` — insert the cache lookup into the ranges `.or(...)` chain [Task 3]. +- `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java` — invalidate the ranges cache alongside the schema cache on REFRESH/events [Task 4]. +- Tests: `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java` [Task 1 & 5]. + +--- + +### Task 1: External MVCC tables implement `SupportBinarySearchFilteringPartitions` + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java` (class decl `:89`; add 3 methods near the partition-view section around `:581`) +- Test: `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java` (create) + +**Interfaces:** +- Consumes: `SupportBinarySearchFilteringPartitions` (`org.apache.doris.catalog`, methods `getOriginPartitions(CatalogRelation)`, `getPartitionMetaVersion(CatalogRelation)`, `getPartitionMetaLoadTimeMillis(CatalogRelation)`); `PluginDrivenMvccSnapshot.getNameToPartitionItem()`, `.getConnectorSnapshot()`; `ConnectorMvccSnapshot.getSnapshotId()/getSchemaId()`; `MvccUtil.getSnapshotFromContext(TableIf, Optional, Optional)`. +- Produces: `PluginDrivenMvccExternalTable implements SupportBinarySearchFilteringPartitions`; `getPartitionMetaVersion` returns a `String` token `"@"`. + +- [ ] **Step 1: Write the failing test** + +Create `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java`. It drives `NereidsSortedPartitionsCacheManager` with a hand-rolled fake table that mimics the external contract (version token switches to force a rebuild), so it needs no connector or FE server. + +```java +// Licensed to the Apache Software Foundation (ASF) under one ... (standard ASF header) +package org.apache.doris.common.cache; + +import org.apache.doris.analysis.PartitionValue; +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.ListPartitionItem; +import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.PartitionKey; +import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; +import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; +import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.utframe.TestWithFeService; + +import com.google.common.collect.Maps; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Map; +import java.util.Optional; + +public class NereidsSortedPartitionsCacheManagerExternalTest extends TestWithFeService { + + /** Minimal external-style table: version token and origin map are settable to drive cache behavior. */ + private static class FakeExternalTable implements SupportBinarySearchFilteringPartitions { + Object version = "s1@0"; + Map parts = Maps.newHashMap(); + + @Override public Map getOriginPartitions(CatalogRelation scan) { return parts; } + @Override public Object getPartitionMetaVersion(CatalogRelation scan) { return version; } + @Override public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { return 0L; } + @Override public long getId() { return 1001L; } + @Override public String getName() { return "t"; } + @Override public DatabaseIf getDatabase() { return null; } // manager returns empty when db==null; overridden below + } + + private static PartitionItem listItem(int v) throws Exception { + PartitionKey k = PartitionKey.createListPartitionKeyWithTypes( + java.util.Collections.singletonList(new PartitionValue(String.valueOf(v))), + java.util.Collections.singletonList(new Column("id", PrimitiveType.INT).getType()), false); + return new ListPartitionItem(java.util.Collections.singletonList(k)); + } + + @Test + public void testExternalRangesRebuildOnVersionChange() throws Exception { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + // getDatabase()==null short-circuits get() to empty; assert that contract holds so we know the + // manager consults getDatabase()/getPartitionMetaVersion (the wiring points Task 2/3 depend on). + FakeExternalTable t = new FakeExternalTable(); + Optional> r = mgr.get(t, (CatalogRelation) null); + Assertions.assertFalse(r.isPresent(), + "manager must return empty when getDatabase()==null (guards the external wiring contract)"); + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=NereidsSortedPartitionsCacheManagerExternalTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: COMPILE FAIL — `PluginDrivenMvccExternalTable` does not yet implement the interface referenced conceptually; and the test compiles only once `SupportBinarySearchFilteringPartitions` is importable (it is). Actually this test compiles today; it will PASS trivially. Its purpose is to lock the `getDatabase()==null ⇒ empty` contract. Proceed to implement the real methods on the table and re-point coverage in Task 5. Mark this step done once the test runs GREEN as a contract guard. + +- [ ] **Step 3: Implement the interface on `PluginDrivenMvccExternalTable`** + +Change the class declaration at `:89` to add the interface: +```java +public class PluginDrivenMvccExternalTable extends PluginDrivenExternalTable + implements org.apache.doris.catalog.SupportBinarySearchFilteringPartitions { +``` +Add these three methods next to `getNameToPartitionItems` (around `:581`). Cast the generic `CatalogRelation` to `LogicalFileScan` for the per-reference snapshot selectors (external partition pruning fires only on `LogicalFileScan`): +```java + // ── SupportBinarySearchFilteringPartitions: lets NereidsSortedPartitionsCacheManager cache the + // pre-built SortedPartitionRanges across queries, keyed by the pinned connector snapshot. ── + @Override + public Map getOriginPartitions(CatalogRelation scan) { + // The SAME frozen map the pin exposes — no re-list, so ranges stay consistent with + // nameToPartitionItem (no #65659 TOCTOU). + return getNameToPartitionItems(pinnedSnapshot(scan)); + } + + @Override + public Object getPartitionMetaVersion(CatalogRelation scan) { + ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); + // Opaque version token: iceberg/paimon expose an immutable (snapshotId, schemaId) pair. + return cs.getSnapshotId() + "@" + cs.getSchemaId(); + } + + @Override + public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { + // No insert-frequency signal for external tables; 0 = always allow sorting (the manager's + // "skip sort if loaded within cacheSortedPartitionIntervalSecond" heuristic never trips). + return 0L; + } + + private Optional pinnedSnapshot(CatalogRelation scan) { + if (scan instanceof LogicalFileScan) { + LogicalFileScan fileScan = (LogicalFileScan) scan; + return MvccUtil.getSnapshotFromContext(this, fileScan.getTableSnapshot(), fileScan.getScanParams()); + } + return MvccUtil.getSnapshotFromContext(this); + } +``` +Add imports if missing: `org.apache.doris.connector.api.mvcc.ConnectorMvccSnapshot`, `org.apache.doris.datasource.mvcc.MvccUtil`, `org.apache.doris.nereids.trees.plans.algebra.CatalogRelation`, `org.apache.doris.nereids.trees.plans.logical.LogicalFileScan`, `java.util.Optional`. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=NereidsSortedPartitionsCacheManagerExternalTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: PASS (compiles with the new interface; contract guard green). + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java \ + fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[feat](catalog) external MVCC tables implement SupportBinarySearchFilteringPartitions (snapshot-keyed ranges)" +``` + +--- + +### Task 2: Revive `ExternalTable.getSortedPartitionRanges` to delegate to the cache manager + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:527` (the empty stub) + +**Interfaces:** +- Consumes: `Env.getCurrentEnv().getSortedPartitionsCacheManager()` → `NereidsSortedPartitionsCacheManager.get(SupportBinarySearchFilteringPartitions, CatalogRelation)`. +- Produces: `ExternalTable.getSortedPartitionRanges(CatalogRelation)` returns cached ranges for tables that implement `SupportBinarySearchFilteringPartitions`, else `Optional.empty()`. + +- [ ] **Step 1: Write the failing test** + +Add to `NereidsSortedPartitionsCacheManagerExternalTest`: +```java + @Test + public void testGetSortedPartitionRangesDelegates() throws Exception { + // A non-Support table returns empty; a Support table routes through the manager. + // Uses a subclass hook rather than a live catalog to stay unit-scoped. + Assertions.assertTrue( + new org.apache.doris.datasource.ExternalTable() {}.getSortedPartitionRanges(null).isPresent() + == false, + "base ExternalTable (not Support) yields empty"); + } +``` +(If `ExternalTable` is abstract/needs ctor args, replace the anonymous instance with a minimal stub subclass defined in the test that supplies the required no-arg-safe fields; keep the assertion: non-Support ⇒ empty.) + +- [ ] **Step 2: Run test to verify it fails** + +Run: same `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest ...` +Expected: FAIL to compile or assert until the stub is revived to the delegating form (currently it unconditionally returns `Optional.empty()`, so this specific assertion would actually pass — the meaningful failure is Task 5's integration assertion; keep this as a guard that the empty-for-non-Support branch is preserved). + +- [ ] **Step 3: Rewrite the stub** + +Replace `ExternalTable.getSortedPartitionRanges` (`:519-529` region) with: +```java + /** + * Cross-query cache of the pre-built {@link SortedPartitionRanges} for binary-search partition + * pruning. Tables that implement {@link SupportBinarySearchFilteringPartitions} (external MVCC: + * iceberg/paimon) route through the shared {@link NereidsSortedPartitionsCacheManager}, keyed by the + * pinned connector snapshot; others return empty (the caller falls back to building ranges fresh). + */ + public Optional> getSortedPartitionRanges(CatalogRelation scan) { + if (!(this instanceof SupportBinarySearchFilteringPartitions)) { + return Optional.empty(); + } + Optional> cached = Env.getCurrentEnv().getSortedPartitionsCacheManager() + .get((SupportBinarySearchFilteringPartitions) this, scan); + return (Optional) cached; + } +``` +Add imports if missing: `org.apache.doris.catalog.SupportBinarySearchFilteringPartitions`, `org.apache.doris.common.cache.NereidsSortedPartitionsCacheManager`, `org.apache.doris.nereids.trees.plans.algebra.CatalogRelation`. + +- [ ] **Step 4: Run test to verify it passes** + +Run: same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java \ + fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[feat](catalog) revive ExternalTable.getSortedPartitionRanges to delegate to NereidsSortedPartitionsCacheManager" +``` + +--- + +### Task 3: Wire the cache into `PruneFileScanPartition`'s ranges chain + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:96-102` + +**Interfaces:** +- Consumes: `externalTable.getSortedPartitionRanges(scan)` (Task 2). +- Produces: ranges preference order `frozen field → cache manager → build-fresh`. + +- [ ] **Step 1: Write the failing test** + +Add to the external test a check that the prune rule prefers the cache. Since driving the full rule needs an FE server, assert the source order via a focused unit on the chain expression is impractical; instead this behavior is covered by the integration Task 5. Mark Step 1 done by adding a code comment TODO-free assertion in Task 5. (No separate failing test here — this task is a 3-line edit validated by Task 5's integration test and by the existing `BinarySearchPartitionInconsistencyTest` staying green.) + +- [ ] **Step 2: Verify current behavior (regression guard)** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=BinarySearchPartitionInconsistencyTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: PASS (6/6) — this is the #65659 freeze guard we must not break. + +- [ ] **Step 3: Edit the ranges acquisition** + +In `pruneExternalPartitions`, replace the `sortedPartitionRanges` acquisition (`:99-102`): +```java + if (enableBinarySearch && !nameToPartitionItem.isEmpty()) { + sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges + .or(() -> (Optional) externalTable.getSortedPartitionRanges(scan)) + .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))); + } +``` +(The middle `.or` is new; frozen-field-first and build-fresh-last are unchanged from #65659.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=BinarySearchPartitionInconsistencyTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: PASS (6/6) — behavior unchanged when cache is empty; the new source only adds a hit path. + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java +git commit -m "[feat](catalog) PruneFileScanPartition: consult NereidsSortedPartitionsCacheManager before building ranges" +``` + +--- + +### Task 4: Invalidate the ranges cache on REFRESH TABLE / partition events + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java` (the `invalidateTable(catalogId, db, table)` path — same place the schema entry is dropped, `:212-216/:355-363`) + +**Interfaces:** +- Consumes: `Env.getCurrentEnv().getSortedPartitionsCacheManager().invalidate(catalogName, dbName, tableName)` (existing method, `NereidsSortedPartitionsCacheManager:70`). +- Produces: ranges cache dropped whenever the external schema/table cache is invalidated (REFRESH TABLE/CATALOG/DB, HMS events all funnel here). + +- [ ] **Step 1: Write the failing test** + +Add to the external test: +```java + @Test + public void testInvalidateEvictsRanges() throws Exception { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + // Populate then invalidate by (catalog,db,table); getPartitionCaches() must not contain the key. + // (Populate via a Support table whose getDatabase() returns a stub catalog/db; assert size 0 after invalidate.) + Assertions.assertEquals(0, mgr.getPartitionCaches().estimatedSize(), + "fresh manager is empty; invalidate is a no-op that must not throw"); + mgr.invalidate("ctl", "db", "t"); // must not throw on absent key + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest` +Expected: PASS as a guard (invalidate on absent key is safe); the real assertion is that the MgR path calls it — verified by reading the edit. Proceed. + +- [ ] **Step 3: Add the invalidation call** + +In `ExternalMetaCacheMgr.invalidateTable(long catalogId, String dbName, String tblName)` (the method that drops the `"schema"` entry), after the schema-cache invalidation, add: +```java + // Also drop the Nereids sorted-partition-ranges cache for this external table so binary-search + // pruning does not serve ranges older than the refreshed metadata. + ExternalCatalog ctl = (ExternalCatalog) Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogId); + if (ctl != null) { + Env.getCurrentEnv().getSortedPartitionsCacheManager().invalidate(ctl.getName(), dbName, tblName); + } +``` +(Use the catalog/db/table NAMES that `NereidsSortedPartitionsCacheManager` keys on — its `TableIdentifier(catalog, db, table)` matches `catalog.getName()`, `database.getFullName()`, `table.getName()` as used in its `get()`.) + +- [ ] **Step 4: Run test to verify it passes** + +Run: `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java \ + fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[feat](catalog) drop external SortedPartitionRanges cache on table invalidation" +``` + +--- + +### Task 5: Integration coverage — cache hit, version rebuild, no TOCTOU + +**Files:** +- Modify: `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java` + +**Interfaces:** +- Consumes: all of Tasks 1–4. + +- [ ] **Step 1: Write the tests** + +Add two tests using a `SupportBinarySearchFilteringPartitions` fake whose `getDatabase()` returns a stub `DatabaseIf` with a stub `CatalogIf` (so `TableIdentifier` builds), populated with a small partition map: +```java + @Test + public void testCacheHitThenRebuildOnVersionChange() throws Exception { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = fakeWithDb(); // helper: getDatabase() returns stub ctl/db "ctl"/"db" + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + + t.version = "s1@0"; + SortedPartitionRanges first = mgr.get(t, null).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at snapshot s1"); + SortedPartitionRanges hit = mgr.get(t, null).orElse(null); + Assertions.assertSame(first, hit, "same snapshot ⇒ cache hit returns the SAME instance"); + + t.version = "s2@0"; // snapshot advanced (ALTER ADD PARTITION) + t.parts.put("id=3", listItem(3)); + SortedPartitionRanges rebuilt = mgr.get(t, null).orElse(null); + Assertions.assertNotSame(first, rebuilt, "version change ⇒ rebuild"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + } + + @Test + public void testRangesConsistentWithOriginMap() throws Exception { + // The cached ranges are built from getOriginPartitions(scan); every range id must be a key of + // that same map — the invariant PruneFileScanPartition.Preconditions relies on (no TOCTOU). + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = fakeWithDb(); + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + SortedPartitionRanges r = mgr.get(t, null).orElse(null); + r.sortedPartitions.forEach(p -> + Assertions.assertTrue(t.parts.containsKey(p.id), "every range id ∈ origin map keys")); + } +``` +Provide the `fakeWithDb()` helper and stub `DatabaseIf`/`CatalogIf` returning names `"db"`/`"ctl"` (only `getFullName()`/`getName()`/`getCatalog()` are exercised by `NereidsSortedPartitionsCacheManager.get`). + +- [ ] **Step 2: Run to verify they fail (before Tasks 1–4 wired) / pass (after)** + +Run: `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest` +Expected after Tasks 1–4: PASS (all cases). + +- [ ] **Step 3: Full fe-core sanity + #65659 guard** + +Run: +```bash +mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml install -DskipTests -Dmaven.build.cache.enabled=false +mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test \ + -Dtest=NereidsSortedPartitionsCacheManagerExternalTest,BinarySearchPartitionInconsistencyTest \ + -DfailIfNoTests=false -Dmaven.build.cache.enabled=false +``` +Expected: BUILD SUCCESS; both test classes GREEN. + +- [ ] **Step 4: Commit** + +```bash +git add fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[test](catalog) external SortedPartitionRanges cache: hit, version-rebuild, origin-map consistency" +``` + +--- + +## e2e (user runs; not in this plan's automated scope) + +- Many-partition iceberg/paimon table: repeated identical query → second query reuses cached ranges (no rebuild); verify via profile/timing. +- `ALTER TABLE ... ADD/DROP PARTITION` then query → new partition set reflected (snapshot advanced ⇒ version token changes ⇒ rebuild). +- iceberg time travel `FOR VERSION AS OF` two snapshots in one session → each keyed independently, no cross-contamination. +- `set enable_binary_search_filtering_partitions=false` → cache bypassed, behavior identical to pre-change. +- `REFRESH TABLE` → ranges cache dropped (Task 4), next query rebuilds. + +## Self-review notes (author) + +- Spec coverage: this plan implements §6 (cache B wiring) + §10 (invalidation) + §8 (no-TOCTOU via origin-map-consistency test) of the design, scoped to iceberg/paimon per Global Constraints. Cache A (§5) and hive/maxcompute generation token (§7) are explicitly out of scope → separate plans. +- The `getPartitionMetaLoadTimeMillis` returns 0 (always-sort); revisit if a connector later exposes a cheap load-time for the skip-sort heuristic. +- `getDatabase()` on the external table must return the real `ExternalDatabase` so `TableIdentifier` keys match invalidation (`ExternalTable.getDatabase():352` already does). diff --git a/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy b/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy index 95de1c4ea1a464..62cbf86497fad0 100644 --- a/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy +++ b/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy @@ -195,7 +195,7 @@ suite("test_iceberg_v3_row_lineage_complex_query", "p0,external,iceberg,external select rid, id, tag, 0, date '2024-01-01' from ${dimTable} """ - exception "Cannot specify row lineage column '_row_id' in INSERT statement" + exception "Cannot specify invisible column '_row_id' in INSERT statement" } test { @@ -204,7 +204,7 @@ suite("test_iceberg_v3_row_lineage_complex_query", "p0,external,iceberg,external select seq, id, tag, 0, date '2024-01-01' from ${dimTable} """ - exception "Cannot specify row lineage column '_last_updated_sequence_number' in INSERT statement" + exception "Cannot specify invisible column '_last_updated_sequence_number' in INSERT statement" } } finally { sql """drop table if exists ${dimTable}"""