ankitsultana commented on code in PR #16242: URL: https://github.com/apache/pinot/pull/16242#discussion_r2183861550
########## pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RelToPlanNodeConverter.java: ########## @@ -94,9 +95,16 @@ public final class RelToPlanNodeConverter { private boolean _windowFunctionFound; @Nullable private final TransformationTracker.Builder<PlanNode, RelNode> _tracker; + private final String _hashFunction; public RelToPlanNodeConverter(@Nullable TransformationTracker.Builder<PlanNode, RelNode> tracker) { Review Comment: isn't this unused? ########## pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/MailboxSendNode.java: ########## @@ -52,22 +54,38 @@ private MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> input _prePartitioned = prePartitioned; _collations = collations != null ? collations : List.of(); _sort = sort; + _hashFunction = hashFunction; } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, @Nullable List<Integer> receiverStages, PinotRelExchangeType exchangeType, RelDistribution.Type distributionType, @Nullable List<Integer> keys, boolean prePartitioned, @Nullable List<RelFieldCollation> collations, boolean sort) { this(stageId, dataSchema, inputs, toBitSet(receiverStages), exchangeType, - distributionType, keys, prePartitioned, collations, sort); + distributionType, keys, prePartitioned, collations, sort, KeySelector.DEFAULT_HASH_ALGORITHM); } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, int receiverStage, PinotRelExchangeType exchangeType, RelDistribution.Type distributionType, @Nullable List<Integer> keys, boolean prePartitioned, @Nullable List<RelFieldCollation> collations, boolean sort) { this(stageId, dataSchema, inputs, toBitSet(receiverStage), exchangeType, distributionType, keys, prePartitioned, - collations, sort); + collations, sort, KeySelector.DEFAULT_HASH_ALGORITHM); + } + + public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, + int receiverStage, PinotRelExchangeType exchangeType, + RelDistribution.Type distributionType, @Nullable List<Integer> keys, boolean prePartitioned, + @Nullable List<RelFieldCollation> collations, boolean sort, String hashFunction) { + this(stageId, dataSchema, inputs, toBitSet(receiverStage), exchangeType, distributionType, keys, prePartitioned, + collations, sort, hashFunction); + } + + public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, Review Comment: there are too many ctors here. this one is only used in a test file. Can you instead update that file to pass in the default hash algorithm? ########## pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/MailboxSendNode.java: ########## @@ -52,22 +54,38 @@ private MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> input _prePartitioned = prePartitioned; _collations = collations != null ? collations : List.of(); _sort = sort; + _hashFunction = hashFunction; } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, @Nullable List<Integer> receiverStages, PinotRelExchangeType exchangeType, RelDistribution.Type distributionType, @Nullable List<Integer> keys, boolean prePartitioned, @Nullable List<RelFieldCollation> collations, boolean sort) { this(stageId, dataSchema, inputs, toBitSet(receiverStages), exchangeType, - distributionType, keys, prePartitioned, collations, sort); + distributionType, keys, prePartitioned, collations, sort, KeySelector.DEFAULT_HASH_ALGORITHM); } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, Review Comment: This is one of the risk with having too many ctors. This one is used in PlanFragmenter in visitExchange. You can pass in the hash function there by adding `node.getHashFunction()` ########## pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/MailboxSendNode.java: ########## @@ -52,22 +54,38 @@ private MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> input _prePartitioned = prePartitioned; _collations = collations != null ? collations : List.of(); _sort = sort; + _hashFunction = hashFunction; } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, Review Comment: This one is used in `PlanNodeDeserializer`. You need to update `MailboxSendNode` in the proto file to take in a hash function and pass it on there. ########## pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/MailboxSendNode.java: ########## @@ -52,22 +54,38 @@ private MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> input _prePartitioned = prePartitioned; _collations = collations != null ? collations : List.of(); _sort = sort; + _hashFunction = hashFunction; } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, @Nullable List<Integer> receiverStages, PinotRelExchangeType exchangeType, RelDistribution.Type distributionType, @Nullable List<Integer> keys, boolean prePartitioned, @Nullable List<RelFieldCollation> collations, boolean sort) { this(stageId, dataSchema, inputs, toBitSet(receiverStages), exchangeType, - distributionType, keys, prePartitioned, collations, sort); + distributionType, keys, prePartitioned, collations, sort, KeySelector.DEFAULT_HASH_ALGORITHM); } public MailboxSendNode(int stageId, DataSchema dataSchema, List<PlanNode> inputs, Review Comment: You can leave `PlanFragmentAndMailboxAssignment` be though. I'll take care of that one (there's a few more things we need to do with v2 optimizer) -- 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