somandal commented on code in PR #10846: URL: https://github.com/apache/pinot/pull/10846#discussion_r1248183763
########## pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotAggregateReduceFunctionsRule.java: ########## @@ -0,0 +1,427 @@ +/** + * 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.calcite.rel.rules; + +import com.google.common.collect.ImmutableSet; +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import org.apache.calcite.plan.RelOptRule; +import org.apache.calcite.plan.RelOptRuleCall; +import org.apache.calcite.rel.core.Aggregate; +import org.apache.calcite.rel.core.AggregateCall; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeFactory; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.PinotSqlAggFunction; +import org.apache.calcite.sql.SqlAggFunction; +import org.apache.calcite.sql.SqlFunction; +import org.apache.calcite.sql.SqlFunctionCategory; +import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.sql.fun.SqlStdOperatorTable; +import org.apache.calcite.tools.RelBuilder; +import org.apache.calcite.tools.RelBuilderFactory; +import org.apache.calcite.util.CompositeList; +import org.apache.calcite.util.Util; +import org.apache.pinot.segment.spi.AggregationFunctionType; + + +/** + * Note: This class copies the logic for reducing SUM and AVG from {@link AggregateReduceFunctionsRule} with some + * changes to use our Pinot defined operand type checkers and return types. This is necessary otherwise the v1 + * aggregations won't work in the v2 engine due to type issues. Once we fix the return types for the v1 aggregation + * functions the logic for AVG and SUM can be removed. We also had to resort to using an AVG_REDUCE scalar function + * due to null handling issues with DIVIDE (returning null on count = 0 via a CASE statement was also not possible + * as the types of the columns were all non-null and Calcite marks nullable and non-nullable columns as incompatible). + * + * We added additional logic to handle typecasting MIN / MAX functions for EVERY / SOME aggregation functions in Calcite + * which internally uses MIN / MAX with boolean return types. This was necessary because the v1 aggregations for + * MIN / MAX always return DOUBLE and this caused type issues for certain queries that utilize Calcite's EVERY / SOME + * aggregation functions. + * + * Planner rule that reduces aggregate functions in + * {@link org.apache.calcite.rel.core.Aggregate}s to simpler forms. + * + * <p>Rewrites: + * <ul> + * + * <li>AVG(x) → SUM(x) / COUNT(x) + * + * </ul> + * + * <p>Since many of these rewrites introduce multiple occurrences of simpler + * forms like {@code COUNT(x)}, the rule gathers common sub-expressions as it + * goes. + * + * @see CoreRules#AGGREGATE_REDUCE_FUNCTIONS + */ +public class PinotAggregateReduceFunctionsRule Review Comment: discussed this already and decided not to rename it back as it was added as a replacement for an existing calcite rule and not a replacement for the other one -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
