oleg-zinovev commented on code in PR #12096: URL: https://github.com/apache/ignite/pull/12096#discussion_r2253963201
########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/logical/ProjectWindowConstantsRule.java: ########## @@ -0,0 +1,183 @@ +/* + * 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.processors.query.calcite.rule.logical; + +import java.util.ArrayList; +import java.util.List; +import com.google.common.collect.ImmutableList; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.plan.RelRule; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Window; +import org.apache.calcite.rel.logical.LogicalWindow; +import org.apache.calcite.rel.rules.TransformationRule; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeFactory; +import org.apache.calcite.rel.type.RelDataTypeField; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.rex.RexInputRef; +import org.apache.calcite.rex.RexLiteral; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexWindowBound; +import org.apache.calcite.rex.RexWindowBounds; +import org.apache.calcite.tools.RelBuilder; +import org.immutables.value.Value; + +/** + * A rule to split window rel with constants to: + * - project with constants + * - window without constants + * - project removing constants + */ +@Value.Enclosing +public class ProjectWindowConstantsRule extends RelRule<ProjectWindowConstantsRule.Config> implements TransformationRule { Review Comment: The implementation of aggregates in Ignite relies on the indices of values in the row instead of evaluating expressions in _AggregateCall#rexList_. In this case, constants must be present in the row before it is processed by the aggregate. If we want to eliminate a separate rule that implements this using projections, I can suggest one of the following options: - Redesign AccumulatorsFactory so that it uses _AggregateCall#rexList_ instead of _AggregateCall#argList_ when computing the aggregate, and replace RexInputRef for window constants with _RexLiteral_ in the _WindowConverterRule_. - Add projection of constants when computing window functions so that the row passed to accumulators contains window constants in the required positions. However, in this case, the implementation of _WindowNode_ will include logic that already exists in _ProjectionNode_. - Implement a separate factory for aggregates (or add new methods in the current one) that works directly with _RexWinAggCall_. In this case, AccumulatorsFactory (or the new factory for window functions) will need to handle a set of RexNode operands, but this will not affect the current logic for regular _AggregateCall_. Also, this would allow abandoning the transformation of _RexWinAggCall_ into _AggregateCall_ and, thus, avoid copying groups in WindowConverterRule. However, in this scenario, I might need to change the visibility of classes in the package _org.apache.ignite.internal.processors.query.calcite.exec.exp.agg_. Let me know if you'd like to propose a more robust solution. -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
