github-actions[bot] commented on code in PR #69277:
URL: https://github.com/apache/doris/pull/69277#discussion_r4312204876
##########
be/src/format_v2/lance/lance_session_manager.cpp:
##########
@@ -231,9 +237,46 @@ Status LanceSessionManager::open_dataset(const char* uri,
const char* const* sto
*dataset = lance_dataset_open_with_session(uri, storage_options, version,
_session);
if (*dataset == nullptr) {
+ // Constructing an SDK error Status can itself log the provider
message. Redact before
+ // that construction, not only when returning the prewarm RPC response.
+ if (redact_errors) {
+ return Status::InternalError(
+ "Unable to open the pinned Lance snapshot for index
prewarm");
+ }
return lance_error("open Lance dataset with shared session");
}
return Status::OK();
}
+Status LanceSessionManager::prewarm_index(const char* uri, const char* const*
storage_options,
+ uint64_t version, const char*
index_name) {
+ if (uri == nullptr || *uri == '\0' || version == 0 || index_name ==
nullptr ||
+ *index_name == '\0') {
+ return Status::InvalidArgument(
+ "Lance index prewarm requires URI, index name and a fixed
version");
+ }
+ // Bound concurrent SDK prewarm IO per BE without blocking all RPC workers
behind a mutex.
+ // A failed or timed-out caller may retry after the existing SDK operation
finishes.
+ std::unique_lock lock(_prewarm_mutex, std::try_to_lock);
+ if (!lock.owns_lock()) {
+ return Status::Cancelled("Another Lance index prewarm is running on
this backend");
+ }
+ LanceDataset* raw_dataset = nullptr;
+ auto status = _open_dataset(uri, storage_options, version, &raw_dataset,
true);
+ std::unique_ptr<LanceDataset, decltype(&lance_dataset_close)>
dataset(raw_dataset,
+
lance_dataset_close);
+ if (!status.ok()) {
+ // SDK storage errors can include credentials or signed URLs. Return
operation context only.
+ return Status::InternalError("Unable to open the pinned Lance snapshot
for index prewarm");
+ }
+ if (lance_dataset_version(dataset.get()) != version) {
+ return Status::InternalError("Lance index prewarm opened an unexpected
snapshot");
+ }
+ if (lance_dataset_prewarm_index(dataset.get(), index_name) != 0) {
Review Comment:
[P2] Disambiguate index names from column names during prewarm
With a valid Lance table containing `id: Int64` and `embedding:
FixedSizeList<Float32>`, create a modern `IVF_FLAT` index on `embedding` named
`id`. FE correctly selects it by field ID, but this call into the pinned SDK
passes `id` to `open_generic_index`; its
[resolve_index_column](https://github.com/lance-format/lance/blob/68c12dfd7efe02f90ad2d7f3a239a7eb64884e57/rust/lance/src/index.rs#L4095-L4123)
prefers the schema column named `id`, and `get_vector_type(..., "id")` then
fails. Thus a cold BE rejects `WARM UP SELECT embedding ...` for a supported
index while indexed searches using `embedding` work. Patch or upgrade the
dependency to resolve the indexed field from segment metadata, and cover this
naming collision with a cold-cache test.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -9604,6 +9626,43 @@ public LogicalPlan
visitWarmUpSelect(DorisParser.WarmUpSelectContext ctx) {
return withExplain(command, ctx.explain());
}
+ private List<String> indexWarmupColumns(DorisParser.WarmUpSelectContext
ctx, List<Expression> projects) {
+ DorisParser.WarmUpSingleTableRefContext table =
ctx.warmUpSingleTableRef();
+ List<String> qualifier =
visitMultipartIdentifier(table.multipartIdentifier());
+ if (table.tableAlias() != null &&
table.tableAlias().strictIdentifier() != null) {
+ if (table.tableAlias().identifierList() != null) {
+ throw new AnalysisException("Index-only WARM UP SELECT does
not support column aliases");
+ }
+ qualifier =
ImmutableList.of(table.tableAlias().strictIdentifier().getText());
+ }
+ List<String> columns = new ArrayList<>();
+ for (Expression expression : projects) {
+ if (!(expression instanceof UnboundSlot) && !(expression
instanceof UnboundStar)) {
+ throw new AnalysisException("Index-only WARM UP SELECT
requires column names or *");
+ }
+ Slot slot = (Slot) expression;
+ List<String> actual = slot.getQualifier();
+ if (actual.size() > qualifier.size()) {
Review Comment:
[P2] Resolve default qualifiers before validating projections
With current catalog `lake` and database `demo`, `WARM UP SELECT
demo.items.embedding FROM items PROPERTIES (read_index_only=true)` is rejected
here because `actual` has two parts but `qualifier` contains only `items`.
Fully qualified stars fail similarly when FROM omits the catalog/database.
These names refer to the same table and the normal binding/data-warmup path
accepts them after `RelationUtil.getQualifierName` fills the defaults. Validate
against the resolved table identity, while retaining alias checks, so
index-only mode accepts these valid projections.
--
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]
