xtern commented on code in PR #7309:
URL: https://github.com/apache/ignite-3/pull/7309#discussion_r2655408303
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/KeyValueGetPlan.java:
##########
@@ -316,4 +316,12 @@ private abstract static class Performable<RowT> {
public int catalogVersion() {
return catalogVersion;
}
+
+ @Override
+ public boolean lazyCursorPublication() {
+ // In order to repeat execution in case of an error where the replica
request schema
Review Comment:
Fixed, thanks
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlPlanToTxSchemaVersionValidator.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.ignite.internal.sql.engine;
+
+import static
org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.catalog.CatalogService;
+import org.apache.ignite.internal.hlc.HybridTimestamp;
+import org.apache.ignite.internal.schema.SchemaSyncService;
+import org.apache.ignite.internal.sql.engine.exec.SqlPlanOutdatedException;
+import org.apache.ignite.internal.sql.engine.prepare.MultiStepPlan;
+import org.apache.ignite.internal.sql.engine.tx.QueryTransactionWrapper;
+import org.apache.ignite.internal.tx.InternalTransaction;
+import org.jetbrains.annotations.TestOnly;
+
+/**
+ * SQL query execution plan validator.
+ *
+ * <p>Performs validation of the catalog version from the {@link MultiStepPlan
multi-step plan} relative to the started transaction.
+ */
+public class SqlPlanToTxSchemaVersionValidator {
+ @TestOnly
+ public static final SqlPlanToTxSchemaVersionValidator NOOP = new
NoopSqlPlanToTxSchemaVersionValidator();
+
+ private final SchemaSyncService schemaSyncService;
+ private final CatalogService catalogService;
+
+ /**
+ * Compares the catalog version from the plan with the version of the
active catalog
+ * at the {@link InternalTransaction#schemaTimestamp() schema time} of the
transaction.
+ *
+ * @param plan {@link MultiStepPlan multi-step} execution plan.
+ * @param tx Query transaction wrapper.
+ * @return Successfully completed future if the provided transaction is
explicit or the catalog versions match.
+ * Otherwise returns a future completed with an exception {@link
SqlPlanOutdatedException}.
+ */
+ public CompletableFuture<Void> validate(MultiStepPlan plan,
QueryTransactionWrapper tx) {
+ if (!tx.implicit()) {
+ return nullCompletedFuture();
+ }
+
+ HybridTimestamp ts = tx.unwrap().schemaTimestamp();
+
+ return schemaSyncService.waitForMetadataCompleteness(ts)
+ .thenRun(() -> {
+ int requiredCatalog =
catalogService.activeCatalogVersion(ts.longValue());
Review Comment:
Added TODO with link to IGNITE-27491
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/SqlPlanOutdatedException.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.ignite.internal.sql.engine.exec;
+
+import org.apache.ignite.internal.lang.IgniteInternalException;
+import org.apache.ignite.internal.tx.InternalTransaction;
+import org.apache.ignite.lang.ErrorGroups.Common;
+
+/**
+ * Exception occurs when SQL engine detects that the execution plan is
outdated after preparation.
+ *
+ * <p>It is used internally to signal the SQL engine to retry the optimization
step
+ * using the transaction {@link InternalTransaction#schemaTimestamp() schema
timestamp}.
+ * This exception should never be passed to the user, and has no special error
code.
+ */
+public class SqlPlanOutdatedException extends IgniteInternalException {
Review Comment:
Thanks, fixed.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/QueryExecutionProgram.java:
##########
@@ -75,6 +80,32 @@ private QueryExecutionProgram() {
static boolean errorHandler(Query query, Throwable th) {
if (canRecover(query, th)) {
query.error.set(null);
+
+ if (multiStepPlanOutdated(th)) {
+ assert query.plan instanceof MultiStepPlan;
+ assert query.currentPhase() ==
ExecutionPhase.CURSOR_INITIALIZATION : query.currentPhase();
+
+ SqlOperationContext context = query.operationContext;
+ QueryTransactionWrapper txWrapper = query.usedTransaction;
+
+ assert context != null;
+ assert txWrapper != null;
+
+ context.getAndSetTxOnRetry(txWrapper);
Review Comment:
Thanks, fixed, but with some caveats:
1. The context is recreated (copied) via a static method of
`SqlOperationContext`, since there's currently no external access to, for
example, `errorListener` .
2. The method to retrieve transaction on retry
(`SqlOperationContext.retryTx()`) has getAndSet(null) semantic to avoid issues
with retry/recovery AFTER re-planning the query.
--
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]
