korlov42 commented on code in PR #6835:
URL: https://github.com/apache/ignite-3/pull/6835#discussion_r2465426803
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/QueryExecutor.java:
##########
@@ -623,6 +623,10 @@ static class ParsedResultWithNextCursorFuture {
this.parsedQuery = parsedQuery;
this.nextCursorFuture = nextCursorFuture;
}
+
+ public ParsedResult parsedQuery() {
Review Comment:
looks like this method is not used
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/DdlBatchingHelper.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.fsm;
+
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+
+/**
+ * Provide helper methods for batched DDL commands.
+ */
+class DdlBatchingHelper {
+ /** Returns {@code true} if commands (represented by AST trees) can be
executed together, {@code false} otherwise. */
+ static boolean isCompatible(SqlNode node1, SqlNode node2) {
+ DdlCommandType kind1 = getCommandType(node1);
+ DdlCommandType kind2 = getCommandType(node2);
+
+ return kind1 == kind2 && kind1 != DdlCommandType.OTHER;
+ }
+
+ /** Returns command kind. */
+ private static DdlCommandType getCommandType(SqlNode node) {
+ SqlKind kind = node.getKind();
+
+ switch (kind) {
Review Comment:
I'm wondering if we can improve discoverability of batch-splitting rules.
Assume we introduce new CREATE statement. How to make it obvious for developer
that some attention is required to make new statement batch-able?
Does't it make sense to introduce new annotation and to require every
top-level AST node to be marked with this annotation? Something like
`@DdlBatchAware(DdlBatchGroup.[CREATE|DROP|OTHER])`. This may be not very
elegant solution, but at least decreases chances of forgetting to handle new
AST during script processing
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImpl.java:
##########
@@ -203,10 +186,27 @@ public int dynamicParamsCount() {
return dynamicParamCount;
}
+ /** {@inheritDoc} */
+ @Override
+ public SqlNode parsedTreeSafe() {
Review Comment:
The idea looks nice, but, I'm afraid, it doesn't work. This method may
return an AST which is changed concurrently by another thread. For current
implementation this is, perhaps, not a big deal as we only interested in
SqlKind of topmost node, but later this may become a source of very obscure
bugs... The problematic sequence of event as follow: thread 1 computes a new
value of tree by invoking `parsedTreeSupplier`, updates the cache and returns
this tree in a context which is only traverse the tree but not changes it; in
the mean time, another thread may request a tree for modification and gets the
very same instance as cachedParsedTree has been just updated an thus not empty
--
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]
