-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make the order of complex grouping key consistent #19103
Make the order of complex grouping key consistent #19103
Conversation
@@ -1156,7 +1156,7 @@ private GroupingSetsPlan planGroupingSets(PlanBuilder subPlan, QuerySpecificatio | |||
groupingSetMappings.put(output, input); | |||
} | |||
|
|||
Map<ScopeAware<Expression>, Symbol> complexExpressions = new HashMap<>(); | |||
Map<ScopeAware<Expression>, Symbol> complexExpressions = new LinkedHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this affect the order of expressions in the plan? If I recall correctly, this is just for doing some mapping later, but they are not used for driving the order in which things get added to the plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is
// combine (cartesian product) with complex expressions
List<List<Symbol>> groupingSets = sets.stream()
.map(set -> ImmutableList.<Symbol>builder()
.addAll(set)
.addAll(complexExpressions.values())
.build())
.collect(toImmutableList());
below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the code that @sopel39 pasted uses complexExpressions.values()
which is not deterministict for HashSet
here is a sample query that shows the indeterminism:
the first run has
|
Description
Additional context and related issues
Release notes
( X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: