github-actions[bot] commented on code in PR #69299:
URL: https://github.com/apache/doris/pull/69299#discussion_r4373756767
##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorTableMetadataOps.java:
##########
@@ -70,6 +70,21 @@ default Optional<ConnectorTableHandle> getTableHandle(
return Optional.empty();
}
+ /**
+ * Resolves a handle for DDL after the caller has checked live table
existence. Connectors whose read handles
+ * require scan-capable metadata may return an identifier-only handle
here. Other connectors retain their
+ * existing handle resolution behavior.
+ */
+ default Optional<ConnectorTableHandle> getTableHandleForDdl(
Review Comment:
[P2] Record the new DDL method in the SPI baseline
Adding this inherited default also adds
`getTableHandleForDdl(org.apache.doris.connector.spi.ConnectorSession,java.lang.String,java.lang.String)`
to `ConnectorMetadata.class.getMethods()`, but
`connector-metadata-methods.txt` still omits it.
`ConnectorMetadataSurfaceTest.methodSurfaceMatchesRecordedBaseline` compares
that complete reflected set with the recorded file, so the SPI unit suite fails
at this head. Add the new signature to the baseline alongside this API change.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -455,26 +469,59 @@ private String loadTableComment(ConnectorSession session,
String dbName, String
@Override
public Optional<ConnectorTableHandle> getTableHandle(
ConnectorSession session, String dbName, String tableName) {
+ IcebergTableHandle handle = new IcebergTableHandle(dbName, tableName);
+ if (IcebergStatementScope.hasReadTable(session, dbName, tableName)
+ || (tableCache != null &&
tableCache.contains(TableIdentifier.of(dbName, tableName)))) {
+ return Optional.of(handle);
+ }
+ // Offline callers cannot retain a cache lease. Pure existence checks
use tableExists instead.
+ if (session == null || session.getStatementScope() ==
ConnectorStatementScope.NONE) {
+ return tableExists(session, dbName, tableName) ?
Optional.of(handle) : Optional.empty();
+ }
+ try {
+ // Loading proves existence and pins the same metadata used by
schema/snapshot/scan planning.
+ // Borrow through the existing generation-aware cache so
concurrent cold readers share one load.
+ executeAuthenticated(() -> resolveTableForRead(session, handle));
Review Comment:
[P2] Preserve metadata-only EXECUTE actions on server-planned tables
With a cold or disabled table cache, `ALTER TABLE t EXECUTE
rollback_to_snapshot('snapshot_id'='<ancestor>')` still enters this new
read-table load through `ConnectorExecuteAction.execute`'s `getTableHandle`
call. A REST table using server-side scan planning is rejected by
`IcebergScanPlanning.rejectServerSideScanPlanning` before the procedure runs,
although this SINGLE_CALL action only uses snapshots and
`manageSnapshots().rollbackTo(...).commit()`. Route metadata-only procedure
targets through `getTableHandleForDdl` too. This is the separate EXECUTE
adapter; the existing DROP/catalog-ALTER thread's callers are already fixed.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenPartitionView.java:
##########
@@ -0,0 +1,309 @@
+// 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.datasource.mvcc;
+
+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.RangePartitionItem;
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.connector.cache.JvmSizeUtils;
+import org.apache.doris.connector.cache.MetaCacheSizeEstimate;
+import org.apache.doris.connector.cache.ReflectiveObjectSizeEstimator;
+import org.apache.doris.connector.spi.ConnectorPartitionInfo;
+import org.apache.doris.connector.spi.mvcc.ConnectorMvccPartition;
+import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges;
+import org.apache.doris.qe.ConnectContext;
+import org.apache.doris.thrift.TScalarType;
+import org.apache.doris.thrift.TTypeDesc;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+
+/** One published partition generation: maps and pruning index share the same
read-only partition items. */
+public final class PluginDrivenPartitionView {
+ private final ImmutableMap<String, PartitionItem> items;
+ private final ImmutableMap<String, Long> freshness;
+ private final SortedPartitionRanges<String> sortedRanges;
+
+ PluginDrivenPartitionView(Map<String, PartitionItem> items, Map<String,
Long> freshness) {
+ this.items = ImmutableMap.copyOf(items);
+ this.freshness = ImmutableMap.copyOf(freshness);
+ // Build before cache admission, like HivePartitionValues: the cache
accounts for the index
+ // together with its maps, and readers never observe a partially
initialized generation.
+ this.sortedRanges = hasComparableKeys(this.items) ?
SortedPartitionRanges.build(this.items) : null;
Review Comment:
[P2] Skip index construction when binary-search pruning is disabled
For a large partitioned Hive/Paimon table with
`enable_binary_search_filtering_partitions=false` and
`meta.cache.default.partition_view.enable=false`, every eager snapshot now
calls `SortedPartitionRanges.build` here, including an unfiltered full scan.
That converts every partition key, allocates an index, and sorts all ranges,
adding O(P log P) work on every query even though both pruning consumers
explicitly refuse to use the index. Previously sorting happened only when
filter pruning requested it behind the session switch. Build the index on
demand or honor the pruning gate before constructing it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
