korlov42 commented on code in PR #4221: URL: https://github.com/apache/ignite-3/pull/4221#discussion_r1721298494
########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteSelectCount.java: ########## @@ -0,0 +1,150 @@ +/* + * 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.rel; + +import java.util.List; +import org.apache.calcite.plan.RelOptCluster; +import org.apache.calcite.plan.RelOptCost; +import org.apache.calcite.plan.RelOptPlanner; +import org.apache.calcite.plan.RelOptTable; +import org.apache.calcite.plan.RelTraitSet; +import org.apache.calcite.rel.AbstractRelNode; +import org.apache.calcite.rel.RelInput; +import org.apache.calcite.rel.RelWriter; +import org.apache.calcite.rel.metadata.RelMetadataQuery; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexUtil; +import org.apache.ignite.internal.sql.engine.exec.mapping.MappingService; +import org.apache.ignite.internal.sql.engine.metadata.cost.IgniteCostFactory; +import org.apache.ignite.internal.sql.engine.util.Commons; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Relational operator that represents a {@code SELECT COUNT(*) FROM table} queries such as: + *<ul> + * <li>{@code SELECT COUNT(*) FROM table}</li> + * <li>{@code SELECT COUNT(not_null_col) FROM table}</li> + * <li>{@code SELECT COUNT(1) FROM table}</li> + *</ul> + * + * <p>Queries without a {@code FROM} clause, can also be presented by this operator: + * <ul> + * <li>{@code SELECT COUNT(*)}</li> + * <li>{@code SELECT COUNT(1)}</li> + * </ul> Review Comment: since `table` is required attribute, this section is not valid anymore ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/PlannerHelper.java: ########## @@ -343,4 +356,145 @@ static boolean hasSubQuery(SqlNode node) { return super.visit(call); } } + + + /** + * Tries to optimize a query that looks like {@code SELECT count(*)}. + * + * @param planner Planner. + * @param txContext Transactional context. + * @param node Query node. + * @return Plan node with list of aliases, if the optimization is applicable. + */ + public static @Nullable Pair<IgniteRel, List<String>> tryOptimizeSelectCount( + IgnitePlanner planner, + @Nullable QueryTransactionContext txContext, + SqlNode node + ) { + if (txContext != null && txContext.explicitTx() != null) { + return null; + } + + if (!(node instanceof SqlSelect)) { + return null; + } + + SqlSelect select = (SqlSelect) node; + + if (select.getGroup() != null + || select.getWhere() != null + || select.getHaving() != null + || select.getQualify() != null + || !select.getWindowList().isEmpty() + || select.getOrderList() != null && !select.getOrderList().isEmpty() Review Comment: I see you've added orderList here. What is wrong with `ORDER BY`? I mean, at first glance ordering doesn't affect result of COUNT. Do I miss something? ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/SelectCountPlan.java: ########## @@ -0,0 +1,184 @@ +/* + * 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.prepare; + +import java.util.Iterator; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.function.BiFunction; +import java.util.function.Function; +import org.apache.calcite.plan.RelOptTable; +import org.apache.calcite.plan.RelOptUtil; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeFactory; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.SqlExplainLevel; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.ignite.internal.logger.IgniteLogger; +import org.apache.ignite.internal.logger.Loggers; +import org.apache.ignite.internal.sql.engine.InternalSqlRow; +import org.apache.ignite.internal.sql.engine.InternalSqlRowImpl; +import org.apache.ignite.internal.sql.engine.QueryPrefetchCallback; +import org.apache.ignite.internal.sql.engine.SqlQueryType; +import org.apache.ignite.internal.sql.engine.exec.ExecutablePlan; +import org.apache.ignite.internal.sql.engine.exec.ExecutableTableRegistry; +import org.apache.ignite.internal.sql.engine.exec.ExecutionContext; +import org.apache.ignite.internal.sql.engine.exec.RowHandler; +import org.apache.ignite.internal.sql.engine.exec.row.RowSchema; +import org.apache.ignite.internal.sql.engine.rel.IgniteRel; +import org.apache.ignite.internal.sql.engine.rel.IgniteSelectCount; +import org.apache.ignite.internal.sql.engine.schema.IgniteTable; +import org.apache.ignite.internal.sql.engine.util.Cloner; +import org.apache.ignite.internal.sql.engine.util.Commons; +import org.apache.ignite.internal.sql.engine.util.TypeUtils; +import org.apache.ignite.internal.tx.InternalTransaction; +import org.apache.ignite.internal.type.NativeTypes; +import org.apache.ignite.internal.util.AsyncCursor; +import org.apache.ignite.internal.util.AsyncWrapper; +import org.apache.ignite.sql.ResultSetMetadata; +import org.jetbrains.annotations.Nullable; + +/** + * Plan representing a COUNT(*) query. + */ +public class SelectCountPlan implements ExplainablePlan, ExecutablePlan { + + private static final IgniteLogger LOG = Loggers.forClass(SelectCountPlan.class); + + private final PlanId id; + + private final int catalogVersion; + + private final IgniteSelectCount selectCountNode; + + private final List<RexNode> expressions; + + private final ResultSetMetadata metadata; + + private final ParameterMetadata parameterMetadata; + + SelectCountPlan( + PlanId id, + int catalogVersion, + IgniteSelectCount getCount, + ResultSetMetadata resultSetMetadata, + ParameterMetadata parameterMetadata + ) { + this.id = id; + this.selectCountNode = getCount; + this.expressions = getCount.expressions(); + this.catalogVersion = catalogVersion; + this.metadata = resultSetMetadata; + this.parameterMetadata = parameterMetadata; + } + + public IgniteSelectCount selectCountNode() { + return selectCountNode; + } + + @Override + public <RowT> AsyncCursor<InternalSqlRow> execute(ExecutionContext<RowT> ctx, @Nullable InternalTransaction tx, + ExecutableTableRegistry tableRegistry, @Nullable QueryPrefetchCallback firstPageReadyCallback) { + + assert tx == null : "SelectCount plan can only run within implicit transaction"; + + RelOptTable optTable = selectCountNode.getTable(); + IgniteTable igniteTable = optTable.unwrap(IgniteTable.class); + assert igniteTable != null; + + CompletableFuture<Long> countFut = tableRegistry.getTable(catalogVersion, igniteTable.id()) Review Comment: ```suggestion CompletableFuture<Long> countFut = tableRegistry.getTable(catalogVersion, igniteTable.id()) ``` ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rel/IgniteSelectCount.java: ########## @@ -0,0 +1,150 @@ +/* + * 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.rel; + +import java.util.List; +import org.apache.calcite.plan.RelOptCluster; +import org.apache.calcite.plan.RelOptCost; +import org.apache.calcite.plan.RelOptPlanner; +import org.apache.calcite.plan.RelOptTable; +import org.apache.calcite.plan.RelTraitSet; +import org.apache.calcite.rel.AbstractRelNode; +import org.apache.calcite.rel.RelInput; +import org.apache.calcite.rel.RelWriter; +import org.apache.calcite.rel.metadata.RelMetadataQuery; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexUtil; +import org.apache.ignite.internal.sql.engine.exec.mapping.MappingService; +import org.apache.ignite.internal.sql.engine.metadata.cost.IgniteCostFactory; +import org.apache.ignite.internal.sql.engine.util.Commons; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Relational operator that represents a {@code SELECT COUNT(*) FROM table} queries such as: + *<ul> + * <li>{@code SELECT COUNT(*) FROM table}</li> + * <li>{@code SELECT COUNT(not_null_col) FROM table}</li> + * <li>{@code SELECT COUNT(1) FROM table}</li> + *</ul> + * + * <p>Queries without a {@code FROM} clause, can also be presented by this operator: + * <ul> + * <li>{@code SELECT COUNT(*)}</li> + * <li>{@code SELECT COUNT(1)}</li> + * </ul> + * + * <p>If a select list includes constant literals (e.g. {@code SELECT COUNT(*), 1, 2 FROM table}), + * then such query can be presented by this operator as well. + * + * <p>Note: This operator can not be executed in transactional context and does not participate in distributed query plan. + * Given that node is not supposed to be a part of distributed query plan, the following parts were + * deliberately omitted:<ul> + * <li>this class doesn't implement {@link SourceAwareIgniteRel}, making it impossible + * to map properly by {@link MappingService}</li> + * <li>de-serialisation constructor is omitted (see {@link ProjectableFilterableTableScan#ProjectableFilterableTableScan(RelInput)} + * as example)</li> + * </ul> + */ +public class IgniteSelectCount extends AbstractRelNode implements IgniteRel { + + private static final String REL_TYPE_NAME = "SelectCount"; + + private final RelOptTable table; + + private final List<RexNode> expressions; + + /** + * Constructor. + * + * @param cluster A cluster this relation belongs to. + * @param traits A set of traits this node satisfies. + * @param table A target table. + * @param expressions Returns a list of expressions returned alongside COUNT(*). + */ + public IgniteSelectCount( + RelOptCluster cluster, + RelTraitSet traits, + RelOptTable table, + List<RexNode> expressions + ) { + super(cluster, traits); + + this.table = table; + this.expressions = expressions; + } + + /** {@inheritDoc} */ + @Override + public RelDataType deriveRowType() { + return RexUtil.createStructType(Commons.typeFactory(getCluster()), expressions); + } + + /** {@inheritDoc} */ + @Override + public RelOptTable getTable() { + return table; + } + + /** {@inheritDoc} */ + @Override + public <T> T accept(IgniteRelVisitor<T> visitor) { + return visitor.visit(this); + } + + /** {@inheritDoc} */ + @Override + public IgniteRel clone(RelOptCluster cluster, List<IgniteRel> inputs) { + assert inputs.isEmpty() : inputs; + + return new IgniteSelectCount(cluster, getTraitSet(), table, expressions); + } + + /** {@inheritDoc} */ + @Override + public RelWriter explainTerms(RelWriter pw) { + RelWriter relWriter = super.explainTerms(pw); + if (table != null) { Review Comment: looks like this `if` statement is not necessary anymore ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImpl.java: ########## @@ -238,26 +241,102 @@ public CompletableFuture<QueryPlan> prepareAsync( .parameters(Commons.arrayToMap(operationContext.parameters())) .build(); - result = prepareAsync0(parsedResult, planningContext); + QueryTransactionContext txContext = operationContext.txContext(); + + // Validate statement + CompletableFuture<ValidStatement> validFut = validateStatement(parsedResult, planningContext); + + // Try optimize to fast + CompletableFuture<FastOptimizationResult> plannedOrValidated = Review Comment: tbh, I didn't get the idea behind introduction of additional container for composite result. it's better to do this: ``` CompletableFuture<ValidStatement> validFut = validateStatement(parsedResult, planningContext); result = validFut.thenCompose(stmt -> { QueryPlan plan = tryOptimizeFast(stmt, planningContext, txContext); // If optimize fast returned a plan, return it. if (plan != null) { return CompletableFuture.completedFuture(plan); } // Otherwise, continue with the regular planning. return prepareAsync0(stmt, planningContext, txContext); }); ``` ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImpl.java: ########## @@ -295,19 +379,36 @@ private CompletableFuture<QueryPlan> prepareExplain(ParsedResult parsedResult, P explicandum ); - CompletableFuture<QueryPlan> result; - switch (queryType) { - case QUERY: - result = prepareQuery(newParsedResult, ctx); - break; - case DML: - result = prepareDml(newParsedResult, ctx); - break; - default: - throw new AssertionError("should not get here"); - } + // Validate statement. + + CompletableFuture<ValidStatement> validFut = validateStatement(newParsedResult, ctx); + + // Try optimize to fast + CompletableFuture<FastOptimizationResult> plannedOrValidated = + validFut.thenApply(stmt -> tryOptimizeFast(stmt, ctx, txContext)); Review Comment: optimization flow during EXPLAIN and regular flow must share the same path. -- 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]
