Gabriel39 commented on code in PR #69277: URL: https://github.com/apache/doris/pull/69277#discussion_r4323188339
########## fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceIndexPrewarm.java: ########## @@ -0,0 +1,274 @@ +// 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.lance; + +import org.apache.doris.analysis.ResourceTypeEnum; +import org.apache.doris.analysis.TableName; +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.ScalarType; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.lance.index.LanceIndexInspectionExecutor; +import org.apache.doris.datasource.lance.index.LanceIndexSegmentGroup; +import org.apache.doris.datasource.lance.metadata.LanceTableMetadata; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.mysql.privilege.PrivPredicate; +import org.apache.doris.proto.InternalService.PLanceIndexPrewarmRequest; +import org.apache.doris.proto.InternalService.PLanceIndexPrewarmResponse; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.ShowResultSet; +import org.apache.doris.qe.ShowResultSetMetaData; +import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.resource.computegroup.ComputeGroup; +import org.apache.doris.rpc.BackendServiceProxy; +import org.apache.doris.system.Backend; +import org.apache.doris.system.BeSelectionPolicy; +import org.apache.doris.thrift.TNetworkAddress; +import org.apache.doris.thrift.TStatusCode; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.OptionalInt; +import java.util.Set; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.BooleanSupplier; + +/** Pins table access and coordinates bounded synchronous prewarm RPCs. */ +public final class LanceIndexPrewarm { + static final int MAX_IN_FLIGHT = 8; + private static final ShowResultSetMetaData RESULT_META = ShowResultSetMetaData.builder() + .addColumn(new Column("Table", ScalarType.createStringType())) + .addColumn(new Column("Index", ScalarType.createStringType())) + .addColumn(new Column("DatasetVersion", ScalarType.BIGINT)) + .addColumn(new Column("BackendCount", ScalarType.INT)) + .addColumn(new Column("ElapsedMs", ScalarType.BIGINT)).build(); + + private LanceIndexPrewarm() { + } + + public static ShowResultSetMetaData getResultSetMetaData() { + return RESULT_META; + } + + public static void run(ConnectContext ctx, StmtExecutor executor, TableNameInfo tableName, + List<String> columns, BooleanSupplier cancelled) throws Exception { + long started = System.nanoTime(); + long elapsedMs = ctx.getStartTime() > 0 ? Math.max(0, System.currentTimeMillis() - ctx.getStartTime()) : 0; + long remainingMs = TimeUnit.SECONDS.toMillis(ctx.getQueryTimeoutS()) - elapsedMs; + long deadline = started + TimeUnit.MILLISECONDS.toNanos(Math.max(0, remainingMs)); + tableName.analyze(ctx); Review Comment: Fixed in [90ed9b4c2a](https://github.com/apache/doris/commit/90ed9b4c2a1d329cbecbb9f889430d4934e5b81a). The parser now retains projection qualifiers and the original FROM identifier. Each execution resolves the table against the current session and validates the qualifiers against that same resolved identity before accessing the table. Alias restrictions remain enforced. Regression coverage parses the complete multi-statement request in A and executes after switching to B: the A-qualified projection is rejected and the B-qualified projection is accepted. Qualified stars and aliases are covered as well. All 113 focused FE tests and FE Checkstyle pass. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java: ########## @@ -9583,6 +9583,28 @@ public LogicalPlan visitWarmUpSelect(DorisParser.WarmUpSelectContext ctx) { } LogicalProject project = new LogicalProject(projectList, filter); + Map<String, String> properties = visitPropertyClause(ctx.properties); + for (String key : properties.keySet()) { + if (!"read_index_only".equals(key)) { + throw new AnalysisException("Unknown WARM UP SELECT property: " + key); + } + } + String indexOnly = properties.getOrDefault("read_index_only", "false"); + if (!"true".equalsIgnoreCase(indexOnly) && !"false".equalsIgnoreCase(indexOnly)) { + throw new AnalysisException("read_index_only must be true or false"); + } + if (Boolean.parseBoolean(indexOnly)) { + if (ctx.whereClause() != null || ctx.explain() != null) { + throw new AnalysisException("Index-only WARM UP SELECT does not support WHERE or EXPLAIN"); + } + // Index cache fills must not execute the data scan or require the data-file cache. + List<String> columns = indexWarmupColumns(ctx, projectList); + UnboundBlackholeSink<?> sink = new UnboundBlackholeSink<>(project, + new UnboundBlackholeSinkContext(true)); + return new WarmupSelectCommand(sink, + new TableNameInfo(visitMultipartIdentifier(ctx.warmUpSingleTableRef().multipartIdentifier())), Review Comment: Fixed in [90ed9b4c2a](https://github.com/apache/doris/commit/90ed9b4c2a1d329cbecbb9f889430d4934e5b81a). Index prewarm retains the original multipart identifier and avoids the TableNameInfo/TableName constructors that apply global lowercasing. Resolution and the string-based privilege check preserve the original table spelling for catalog lookup. The regression sets global lowercasing and verifies that Items still reaches the execution-time table access boundary as Items. It failed against the previous implementation and now passes. All 113 focused FE tests and FE Checkstyle pass. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/WarmupSelectCommand.java: ########## @@ -216,10 +231,26 @@ public void sendAggregatedBlackholeResults(Map<Long, TQueryStatisticsResult> sta executor.sendResultSet(resultSet); } + @Override + public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { + if (indexTable != null) { + LanceIndexPrewarm.run(ctx, executor, indexTable, indexColumns, Review Comment: Fixed in [90ed9b4c2a](https://github.com/apache/doris/commit/90ed9b4c2a1d329cbecbb9f889430d4934e5b81a). The command stores immutable original identifier parts and calls RelationUtil.getQualifierName for each execution, keeping the resolved name local to that execution. It no longer mutates and reuses a TableNameInfo with cached defaults. Tests execute the same parsed command repeatedly while changing the current database and catalog; each execution observes the new defaults. The database-switch regression failed before the fix. All 113 focused FE tests and FE Checkstyle pass. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceIndexPrewarm.java: ########## @@ -0,0 +1,274 @@ +// 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.lance; + +import org.apache.doris.analysis.ResourceTypeEnum; +import org.apache.doris.analysis.TableName; +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.ScalarType; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.lance.index.LanceIndexInspectionExecutor; +import org.apache.doris.datasource.lance.index.LanceIndexSegmentGroup; +import org.apache.doris.datasource.lance.metadata.LanceTableMetadata; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.mysql.privilege.PrivPredicate; +import org.apache.doris.proto.InternalService.PLanceIndexPrewarmRequest; +import org.apache.doris.proto.InternalService.PLanceIndexPrewarmResponse; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.ShowResultSet; +import org.apache.doris.qe.ShowResultSetMetaData; +import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.resource.computegroup.ComputeGroup; +import org.apache.doris.rpc.BackendServiceProxy; +import org.apache.doris.system.Backend; +import org.apache.doris.system.BeSelectionPolicy; +import org.apache.doris.thrift.TNetworkAddress; +import org.apache.doris.thrift.TStatusCode; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.OptionalInt; +import java.util.Set; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.BooleanSupplier; + +/** Pins table access and coordinates bounded synchronous prewarm RPCs. */ +public final class LanceIndexPrewarm { + static final int MAX_IN_FLIGHT = 8; + private static final ShowResultSetMetaData RESULT_META = ShowResultSetMetaData.builder() + .addColumn(new Column("Table", ScalarType.createStringType())) + .addColumn(new Column("Index", ScalarType.createStringType())) + .addColumn(new Column("DatasetVersion", ScalarType.BIGINT)) + .addColumn(new Column("BackendCount", ScalarType.INT)) + .addColumn(new Column("ElapsedMs", ScalarType.BIGINT)).build(); + + private LanceIndexPrewarm() { + } + + public static ShowResultSetMetaData getResultSetMetaData() { + return RESULT_META; + } + + public static void run(ConnectContext ctx, StmtExecutor executor, TableNameInfo tableName, + List<String> columns, BooleanSupplier cancelled) throws Exception { + long started = System.nanoTime(); + long elapsedMs = ctx.getStartTime() > 0 ? Math.max(0, System.currentTimeMillis() - ctx.getStartTime()) : 0; + long remainingMs = TimeUnit.SECONDS.toMillis(ctx.getQueryTimeoutS()) - elapsedMs; + long deadline = started + TimeUnit.MILLISECONDS.toNanos(Math.max(0, remainingMs)); + tableName.analyze(ctx); + checkPrivileges(ctx, tableName); + List<Backend> targets = selectBackends(resolveComputeGroup(ctx)); + // Resolve catalog/table initialization and snapshot IO in the bounded metadata pool. + // The worker owns native resources until it returns, even if this caller stops waiting. + String catalogName = tableName.getCtl(); + String databaseName = tableName.getDb(); + String name = tableName.getTbl(); + LanceTableMetadata metadata = LanceIndexInspectionExecutor.execute(() -> { + CatalogIf<?> catalog = Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName); + if (!(catalog instanceof LanceExternalCatalog)) { + throw new AnalysisException("Index-only WARM UP SELECT requires a Lance catalog table"); + } + TableIf table = catalog.getDbOrAnalysisException(databaseName).getTableOrAnalysisException(name); + if (!(table instanceof LanceExternalTable)) { + throw new AnalysisException("Index-only WARM UP SELECT requires a Lance catalog table"); + } + checkActive(deadline, cancelled); + // Use the query access path, including REST-vended credentials, exactly once. + return ((LanceExternalTable) table).loadMetadata(); Review Comment: Fixed in [90ed9b4c2a](https://github.com/apache/doris/commit/90ed9b4c2a1d329cbecbb9f889430d4934e5b81a). Prewarm now has a dedicated metadata path reading raw user-index names and field IDs from the same pinned query-access dataset snapshot. It does not require index_details or reuse the planner-specific type filtering. Duplicate physical segments are merged by logical index name, internal indexes are excluded, and discovery bounds are retained. Planner discovery is unchanged. Tests cover legacy-only and mixed legacy/modern metadata for both star and column selection, duplicate segments, internal indexes, malformed metadata, bounds, and pinned access/version propagation. The legacy-only regression failed before the fix. All 113 focused FE tests and FE Checkstyle pass. ########## thirdparty/download-thirdparty.sh: ########## @@ -718,25 +718,29 @@ if [[ " ${TP_ARCHIVES[*]} " =~ " AZURE " ]]; then echo "Finished patching ${AZURE_SOURCE}" fi -# Foyer remains a local patch until its cache interface is accepted upstream. -# All search fixes are supplied by the immutable lance-c dependency revision. +# Keep local cache integration and prewarm field resolution on the pinned lance-c revision. if [[ " ${TP_ARCHIVES[*]} " =~ " LANCE_C " ]]; then - foyer_patch_checksum="$(cksum < "${TP_PATCH_DIR}/lance-c-foyer.patch")" - foyer_patch_marker="${TP_SOURCE_DIR}/${LANCE_C_SOURCE}/${PATCHED_MARK}_foyer" - # A new local patch must also replace previously patched cached sources. - # Empty markers from older builds cannot identify the applied patch version. - if [[ -f "${foyer_patch_marker}" ]] && - [[ "$(cat "${foyer_patch_marker}")" != "${foyer_patch_checksum}" ]]; then + lance_c_patches=(lance-c-foyer.patch lance-c-prewarm-field.patch) + lance_c_patch_checksum="$( + for patch_file in "${lance_c_patches[@]}"; do + cat "${TP_PATCH_DIR}/${patch_file}" + done | cksum + )" + lance_c_patch_marker="${TP_SOURCE_DIR}/${LANCE_C_SOURCE}/${PATCHED_MARK}_lance_c" Review Comment: Addressed by the upstream dependency replacement in [d594a051ad](https://github.com/apache/doris/commit/d594a051ad432ac4616cf97d67994fa687040430). Lance-C is now pinned to upstream 4fafcd64525b274b113208390bbcca86bbbc093b, which includes the prewarm fix. The local prewarm patch and combined-marker downloader changes were removed, restoring the existing Foyer-only marker protocol that this harness validates. I reran `bash thirdparty/test/lance-prefilter-patch-test.sh thirdparty/src` on the updated branch: all six checks pass, including fresh extraction/idempotence, re-extraction, stale-marker handling, and patch-failure handling. A combined-marker migration is therefore no longer needed. -- 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]
