You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "zabetak (via GitHub)" <gi...@apache.org> on 2024/12/20 22:26:50 UTC
[PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
zabetak opened a new pull request, #5586:
URL: https://github.com/apache/hive/pull/5586
(no comment)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5586:
URL: https://github.com/apache/hive/pull/5586#issuecomment-2557871790
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5586) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5586&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5586&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5586&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5586&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5586&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5586)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1901768007
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java:
##########
@@ -502,28 +502,15 @@ private ImmutableBitSet generateNewGroupset(Aggregate aggregate, ImmutableBitSet
return generateGroupSetIfCardinalitySame(aggregate, originalGroupSet, fieldsUsed);
}
- // we have set of unique key, get to the key which is same as group by key
- ImmutableBitSet groupByUniqueKey = null;
-
+ // Find the maximum number of columns that can be removed by retaining a certain unique key
+ ImmutableBitSet columnsToRemove = ImmutableBitSet.of();
for (ImmutableBitSet key : uniqueKeys) {
- if (aggregate.getGroupSet().contains(key)) {
- groupByUniqueKey = key;
- break;
+ ImmutableBitSet removableCols = originalGroupSet.except(key).except(fieldsUsed);
Review Comment:
Yes, it is possible to compute `except(fieldsUsed)` outside the loop. I applied the suggestion in https://github.com/apache/hive/pull/5586/commits/e89386dc64bd4eb8a3981795b3cacd264861ee35.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5586:
URL: https://github.com/apache/hive/pull/5586#issuecomment-2569263184
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5586) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5586&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5586&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5586&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5586&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5586&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5586)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1903937243
##########
ql/src/test/queries/clientpositive/cbo_groupby_remove_key.q:
##########
@@ -0,0 +1,18 @@
+CREATE TABLE passenger
+(
+ id INT NOT NULL,
+ fname STRING NOT NULL,
+ lname STRING NOT NULL,
+ passport STRING NOT NULL,
+ UNIQUE (id) DISABLE RELY,
+ UNIQUE (passport) DISABLE RELY,
+ UNIQUE (fname, lname) DISABLE RELY
+);
+
+EXPLAIN CBO SELECT id, COUNT(1) FROM passenger GROUP BY id, passport;
+EXPLAIN CBO SELECT passport, COUNT(1) FROM passenger GROUP BY id, passport;
+EXPLAIN CBO SELECT id, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT passport, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT fname, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT lname, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT fname, lname, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
Review Comment:
If we change the aggregate function the query changes as well.
If we assume that the set of redundant columns remains the same after changing the aggregate function (e.g., `MAX(fname)`) then it's always a good idea to drop unnecessary processing so grouping on `fname, lname` is the best option.
If the set of redundant columns changes (e.g., `MAX(passport)`) then this optimization will not trigger in the same way (`passport` column cannot be dropped).
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "ramesh0201 (via GitHub)" <gi...@apache.org>.
ramesh0201 commented on PR #5586:
URL: https://github.com/apache/hive/pull/5586#issuecomment-2569787045
LGTM +1
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak closed pull request #5586: HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses
URL: https://github.com/apache/hive/pull/5586
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1901789987
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java:
##########
@@ -502,28 +502,15 @@ private ImmutableBitSet generateNewGroupset(Aggregate aggregate, ImmutableBitSet
return generateGroupSetIfCardinalitySame(aggregate, originalGroupSet, fieldsUsed);
}
- // we have set of unique key, get to the key which is same as group by key
- ImmutableBitSet groupByUniqueKey = null;
-
+ // Find the maximum number of columns that can be removed by retaining a certain unique key
+ ImmutableBitSet columnsToRemove = ImmutableBitSet.of();
for (ImmutableBitSet key : uniqueKeys) {
- if (aggregate.getGroupSet().contains(key)) {
- groupByUniqueKey = key;
- break;
+ ImmutableBitSet removableCols = originalGroupSet.except(key).except(fieldsUsed);
Review Comment:
Comparing the efficiency of the iteration between `fieldsUsed` and `uniqueKeys` is not possible because the semantics are different. The `uniqueKeys` variable is a set of sets (`Set<ImmutableBitSet>`) while `fieldsUsed` is a single set (`ImmutableBitSet`). Note that a `key` is not necessarily a single column but a set of columns (composite key).
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "ramesh0201 (via GitHub)" <gi...@apache.org>.
ramesh0201 commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1901425607
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java:
##########
@@ -502,28 +502,15 @@ private ImmutableBitSet generateNewGroupset(Aggregate aggregate, ImmutableBitSet
return generateGroupSetIfCardinalitySame(aggregate, originalGroupSet, fieldsUsed);
}
- // we have set of unique key, get to the key which is same as group by key
- ImmutableBitSet groupByUniqueKey = null;
-
+ // Find the maximum number of columns that can be removed by retaining a certain unique key
+ ImmutableBitSet columnsToRemove = ImmutableBitSet.of();
for (ImmutableBitSet key : uniqueKeys) {
- if (aggregate.getGroupSet().contains(key)) {
- groupByUniqueKey = key;
- break;
+ ImmutableBitSet removableCols = originalGroupSet.except(key).except(fieldsUsed);
Review Comment:
Just realized that we only update columnsToRemove when there is atleast one match. So I think yes we can move the except(fieldsUsed) outside the for loop.
Also is it efficient to loop the fieldsUsed(and then check if there is a field that is a unique key and part of aggregate keys to retain only fieldsUsed) vs looping uniquekeys? Mostly depends on size of fieldsUsed vs uniqueKeys. I will probably leave it upto you to decide on that.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "soumyakanti3578 (via GitHub)" <gi...@apache.org>.
soumyakanti3578 commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1901230334
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java:
##########
@@ -502,28 +502,15 @@ private ImmutableBitSet generateNewGroupset(Aggregate aggregate, ImmutableBitSet
return generateGroupSetIfCardinalitySame(aggregate, originalGroupSet, fieldsUsed);
}
- // we have set of unique key, get to the key which is same as group by key
- ImmutableBitSet groupByUniqueKey = null;
-
+ // Find the maximum number of columns that can be removed by retaining a certain unique key
+ ImmutableBitSet columnsToRemove = ImmutableBitSet.of();
for (ImmutableBitSet key : uniqueKeys) {
- if (aggregate.getGroupSet().contains(key)) {
- groupByUniqueKey = key;
- break;
+ ImmutableBitSet removableCols = originalGroupSet.except(key).except(fieldsUsed);
Review Comment:
Is it possible to compute `except(fieldsUsed)` outside the loop? I believe changing the order of `except`s will still yield the same resulting set.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "ramesh0201 (via GitHub)" <gi...@apache.org>.
ramesh0201 commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1901403277
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java:
##########
@@ -502,28 +502,15 @@ private ImmutableBitSet generateNewGroupset(Aggregate aggregate, ImmutableBitSet
return generateGroupSetIfCardinalitySame(aggregate, originalGroupSet, fieldsUsed);
}
- // we have set of unique key, get to the key which is same as group by key
- ImmutableBitSet groupByUniqueKey = null;
-
+ // Find the maximum number of columns that can be removed by retaining a certain unique key
+ ImmutableBitSet columnsToRemove = ImmutableBitSet.of();
for (ImmutableBitSet key : uniqueKeys) {
- if (aggregate.getGroupSet().contains(key)) {
- groupByUniqueKey = key;
- break;
+ ImmutableBitSet removableCols = originalGroupSet.except(key).except(fieldsUsed);
Review Comment:
Yes we can do this, but just want to be careful that if there is no uniquekey match then we should not remove any columns at all(should skip the except in the return statement below in that case)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "ramesh0201 (via GitHub)" <gi...@apache.org>.
ramesh0201 commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1902145217
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java:
##########
@@ -502,28 +502,15 @@ private ImmutableBitSet generateNewGroupset(Aggregate aggregate, ImmutableBitSet
return generateGroupSetIfCardinalitySame(aggregate, originalGroupSet, fieldsUsed);
}
- // we have set of unique key, get to the key which is same as group by key
- ImmutableBitSet groupByUniqueKey = null;
-
+ // Find the maximum number of columns that can be removed by retaining a certain unique key
+ ImmutableBitSet columnsToRemove = ImmutableBitSet.of();
for (ImmutableBitSet key : uniqueKeys) {
- if (aggregate.getGroupSet().contains(key)) {
- groupByUniqueKey = key;
- break;
+ ImmutableBitSet removableCols = originalGroupSet.except(key).except(fieldsUsed);
Review Comment:
Thank you for your explanation. Makes sense to me.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28675: Maximize the removal of redundant columns from GROUP BY clauses [hive]
Posted by "ramesh0201 (via GitHub)" <gi...@apache.org>.
ramesh0201 commented on code in PR #5586:
URL: https://github.com/apache/hive/pull/5586#discussion_r1902146641
##########
ql/src/test/queries/clientpositive/cbo_groupby_remove_key.q:
##########
@@ -0,0 +1,18 @@
+CREATE TABLE passenger
+(
+ id INT NOT NULL,
+ fname STRING NOT NULL,
+ lname STRING NOT NULL,
+ passport STRING NOT NULL,
+ UNIQUE (id) DISABLE RELY,
+ UNIQUE (passport) DISABLE RELY,
+ UNIQUE (fname, lname) DISABLE RELY
+);
+
+EXPLAIN CBO SELECT id, COUNT(1) FROM passenger GROUP BY id, passport;
+EXPLAIN CBO SELECT passport, COUNT(1) FROM passenger GROUP BY id, passport;
+EXPLAIN CBO SELECT id, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT passport, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT fname, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT lname, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
+EXPLAIN CBO SELECT fname, lname, COUNT(1) FROM passenger GROUP BY id, fname, lname, passport;
Review Comment:
In this case, is having group by fname,lname always a better plan? -- even if there is a different aggregate function?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org