From 4ec96f55b2047ac71a75161e2b7993f4184a945d Mon Sep 17 00:00:00 2001 From: Sreeni Viswanadha Date: Mon, 26 Jan 2026 11:54:27 -0800 Subject: [PATCH] fix(optimizer): Use a working hash function for prefilter groupby limit that worns both in java and cpp --- .../presto/sql/planner/PlannerUtils.java | 16 ++++++++++++ .../planner/optimizations/JoinPrefilter.java | 25 +++---------------- .../PrefilterForLimitingAggregation.java | 6 ++--- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java index 4e32a6e6918ed..4285cd778fa01 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java @@ -84,6 +84,7 @@ import static com.facebook.presto.sql.planner.iterative.Lookup.noLookup; import static com.facebook.presto.sql.planner.optimizations.PlanNodeSearcher.searchFrom; import static com.facebook.presto.sql.relational.Expressions.call; +import static com.facebook.presto.sql.relational.Expressions.callOperator; import static com.facebook.presto.sql.relational.Expressions.constant; import static com.facebook.presto.sql.relational.Expressions.variable; import static com.facebook.presto.type.TypeUtils.NULL_HASH_CODE; @@ -577,4 +578,19 @@ public static RowExpression randomizeJoinKey(Session session, FunctionAndTypeMan } return new SpecialFormExpression(COALESCE, VARCHAR, ImmutableList.of(castToVarchar, concatExpression)); } + + public static RowExpression getVariableHash(List inputVariables, FunctionAndTypeManager functionAndTypeManager) + { + checkArgument(!inputVariables.isEmpty()); + List hashExpressionList = inputVariables.stream().map(keyVariable -> + callOperator(functionAndTypeManager.getFunctionAndTypeResolver(), OperatorType.XX_HASH_64, BIGINT, keyVariable)).collect(toImmutableList()); + RowExpression hashExpression = hashExpressionList.get(0); + if (hashExpressionList.size() > 1) { + hashExpression = orNullHashCode(hashExpression); + for (int i = 1; i < hashExpressionList.size(); ++i) { + hashExpression = call(functionAndTypeManager, "combine_hash", BIGINT, hashExpression, orNullHashCode(hashExpressionList.get(i))); + } + } + return hashExpression; + } } diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/JoinPrefilter.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/JoinPrefilter.java index c590bf6a6cb1f..1f3581d29233f 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/JoinPrefilter.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/JoinPrefilter.java @@ -14,7 +14,6 @@ package com.facebook.presto.sql.planner.optimizations; import com.facebook.presto.Session; -import com.facebook.presto.common.function.OperatorType; import com.facebook.presto.common.type.VarcharType; import com.facebook.presto.metadata.FunctionAndTypeManager; import com.facebook.presto.metadata.Metadata; @@ -27,7 +26,6 @@ import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.plan.PlanNodeIdAllocator; import com.facebook.presto.spi.plan.SemiJoinNode; -import com.facebook.presto.spi.relation.CallExpression; import com.facebook.presto.spi.relation.RowExpression; import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.sql.planner.TypeProvider; @@ -42,7 +40,6 @@ import java.util.stream.IntStream; import static com.facebook.presto.SystemSessionProperties.isJoinPrefilterEnabled; -import static com.facebook.presto.common.type.BigintType.BIGINT; import static com.facebook.presto.common.type.BooleanType.BOOLEAN; import static com.facebook.presto.common.type.VarcharType.VARCHAR; import static com.facebook.presto.spi.plan.AggregationNode.singleGroupingSet; @@ -50,13 +47,11 @@ import static com.facebook.presto.spi.plan.JoinType.LEFT; import static com.facebook.presto.sql.planner.PlannerUtils.addProjections; import static com.facebook.presto.sql.planner.PlannerUtils.clonePlanNode; +import static com.facebook.presto.sql.planner.PlannerUtils.getVariableHash; import static com.facebook.presto.sql.planner.PlannerUtils.isScanFilterProject; -import static com.facebook.presto.sql.planner.PlannerUtils.orNullHashCode; import static com.facebook.presto.sql.planner.PlannerUtils.projectExpressions; import static com.facebook.presto.sql.planner.PlannerUtils.restrictOutput; import static com.facebook.presto.sql.planner.plan.ChildReplacer.replaceChildren; -import static com.facebook.presto.sql.relational.Expressions.call; -import static com.facebook.presto.sql.relational.Expressions.callOperator; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static java.util.Objects.requireNonNull; @@ -208,7 +203,7 @@ public PlanNode visitJoin(JoinNode node, RewriteContext context) PlanNode leftKeys = clonePlanNode(rewrittenLeft, session, metadata, idAllocator, leftKeyList, leftVarMap); ImmutableList.Builder expressionsToProject = ImmutableList.builder(); if (hashJoinKey) { - RowExpression hashExpression = getVariableHash(leftKeyList); + RowExpression hashExpression = getVariableHash(leftKeyList, functionAndTypeManager); expressionsToProject.add(hashExpression); } else { @@ -218,7 +213,7 @@ public PlanNode visitJoin(JoinNode node, RewriteContext context) VariableReferenceExpression rightKeyToFilter = rightKeyList.get(0); if (hashJoinKey) { - RowExpression hashExpression = getVariableHash(rightKeyList); + RowExpression hashExpression = getVariableHash(rightKeyList, functionAndTypeManager); rightKeyToFilter = variableAllocator.newVariable(hashExpression); rewrittenRight = addProjections(rewrittenRight, idAllocator, ImmutableMap.of(rightKeyToFilter, hashExpression)); } @@ -273,19 +268,5 @@ public boolean isPlanChanged() { return planChanged; } - - private RowExpression getVariableHash(List inputVariables) - { - List hashExpressionList = inputVariables.stream().map(keyVariable -> - callOperator(functionAndTypeManager.getFunctionAndTypeResolver(), OperatorType.XX_HASH_64, BIGINT, keyVariable)).collect(toImmutableList()); - RowExpression hashExpression = hashExpressionList.get(0); - if (hashExpressionList.size() > 1) { - hashExpression = orNullHashCode(hashExpression); - for (int i = 1; i < hashExpressionList.size(); ++i) { - hashExpression = call(functionAndTypeManager, "combine_hash", BIGINT, hashExpression, orNullHashCode(hashExpressionList.get(i))); - } - } - return hashExpression; - } } } diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PrefilterForLimitingAggregation.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PrefilterForLimitingAggregation.java index 492df9b19fc99..ebbfa738a0417 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PrefilterForLimitingAggregation.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/PrefilterForLimitingAggregation.java @@ -57,8 +57,8 @@ import static com.facebook.presto.sql.planner.PlannerUtils.addProjections; import static com.facebook.presto.sql.planner.PlannerUtils.clonePlanNode; import static com.facebook.presto.sql.planner.PlannerUtils.createMapType; -import static com.facebook.presto.sql.planner.PlannerUtils.getHashExpression; import static com.facebook.presto.sql.planner.PlannerUtils.getTableScanNodeWithOnlyFilterAndProject; +import static com.facebook.presto.sql.planner.PlannerUtils.getVariableHash; import static com.facebook.presto.sql.planner.PlannerUtils.projectExpressions; import static com.facebook.presto.sql.planner.optimizations.JoinNodeUtils.typeConvert; import static com.facebook.presto.sql.planner.plan.ChildReplacer.replaceChildren; @@ -223,8 +223,8 @@ private PlanNode addPrefilter(AggregationNode aggregationNode, long count) SystemSessionProperties.getPrefilterForGroupbyLimitTimeoutMS(session)); FunctionAndTypeManager functionAndTypeManager = metadata.getFunctionAndTypeManager(); - RowExpression leftHashExpression = getHashExpression(functionAndTypeManager, keys).get(); - RowExpression rightHashExpression = getHashExpression(functionAndTypeManager, timedDistinctLimitNode.getOutputVariables()).get(); + RowExpression leftHashExpression = getVariableHash(keys, functionAndTypeManager); + RowExpression rightHashExpression = getVariableHash(timedDistinctLimitNode.getOutputVariables(), functionAndTypeManager); Type mapType = createMapType(functionAndTypeManager, BIGINT, BOOLEAN); PlanNode rightProjectNode = projectExpressions(timedDistinctLimitNode, idAllocator, variableAllocator, ImmutableList.of(rightHashExpression, constant(TRUE, BOOLEAN)), ImmutableList.of());