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 2023/04/20 16:30:16 UTC
[GitHub] [hive] zabetak opened a new pull request, #4253: HIVE-27278: Simplify correlated queries with empty inputs
zabetak opened a new pull request, #4253:
URL: https://github.com/apache/hive/pull/4253
### What changes were proposed in this pull request?
1. Add new pruning rules to remove correlate when inputs are empty.
2. Refactor pruning rules for join to remove code duplication with the addition of new rules.
### Why are the changes needed?
Avoid redundant occurrences of `LogicalCorrelate` in the plan that cannot be handled by the compiler and may lead to exceptions and complation failures.
### Does this PR introduce _any_ user-facing change?
Fixes some queries that fail at compile time.
```sql
EXPLAIN CBO SELECT id FROM t1 WHERE NULL IN (SELECT NULL FROM t2 where t1.id = t2.id);
```
**Before**
```
HiveProject(id=[$0])
LogicalCorrelate(correlation=[$cor0], joinType=[semi], requiredColumns=[{}])
HiveTableScan(table=[[default, t1]], table:alias=[t1])
HiveValues(tuples=[[]])
```
**After**
```
HiveValues(tuples=[[]])
```
### How was this patch tested?
```
mvn test -Dtest=TestMiniLlapLocalCliDriver -Dqfile_regex=.*subquery.*
mvn test -Dtest=TestMiniLlapLocalCliDriver -Dqfile_regex=empty.*
```
--
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
[GitHub] [hive] zabetak closed pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak closed pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
URL: https://github.com/apache/hive/pull/4253
--
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
[GitHub] [hive] kasakrisz commented on a diff in pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4253:
URL: https://github.com/apache/hive/pull/4253#discussion_r1176138739
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
+ int padding = resultType.getFieldCount() - input.getRowType().getFieldCount();
+ List<RexNode> nullLiterals = Collections.nCopies(padding, builder.literal(null));
+ builder.push(input);
+ if (leftPadding) {
+ builder.project(concat(nullLiterals, builder.fields()));
+ } else {
+ builder.project(concat(builder.fields(), nullLiterals));
+ }
+ return builder.convert(resultType, true).build();
+ }
+
+ public static final RelOptRule CORRELATE_RIGHT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(RelNode.class).anyInputs(),
+ b2 -> b2.operand(Values.class).predicate(Values::isEmpty).noInputs()))
+ .withDescription("PruneEmptyCorrelate(right)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+ public static final RelOptRule CORRELATE_LEFT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(Values.class).predicate(Values::isEmpty).noInputs(),
+ b2 -> b2.operand(RelNode.class).anyInputs()))
+ .withDescription("PruneEmptyCorrelate(left)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+
+ /** Configuration for rule that prunes a correlate if one of its inputs is empty. */
+ public interface CorrelateEmptyRuleConfig extends PruneEmptyRule.Config {
Review Comment:
nit: should the implementation of this class go into two separate classes since there is no common parts of handling left and right empty inputs?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
Review Comment:
nit: you can get rid of the flag argument `leftPadding` by creating two methods each of them responsible only for one type of padding:
```
leftPadWithNulls(RelBuilder builder, RelNode input, RelDataType resultType)
rightPadWithNulls(RelBuilder builder, RelNode input, RelDataType resultType)
```
--
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
[GitHub] [hive] zabetak commented on a diff in pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4253:
URL: https://github.com/apache/hive/pull/4253#discussion_r1176303967
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
+ int padding = resultType.getFieldCount() - input.getRowType().getFieldCount();
+ List<RexNode> nullLiterals = Collections.nCopies(padding, builder.literal(null));
+ builder.push(input);
+ if (leftPadding) {
+ builder.project(concat(nullLiterals, builder.fields()));
+ } else {
+ builder.project(concat(builder.fields(), nullLiterals));
+ }
+ return builder.convert(resultType, true).build();
+ }
+
+ public static final RelOptRule CORRELATE_RIGHT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(RelNode.class).anyInputs(),
+ b2 -> b2.operand(Values.class).predicate(Values::isEmpty).noInputs()))
+ .withDescription("PruneEmptyCorrelate(right)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+ public static final RelOptRule CORRELATE_LEFT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(Values.class).predicate(Values::isEmpty).noInputs(),
+ b2 -> b2.operand(RelNode.class).anyInputs()))
+ .withDescription("PruneEmptyCorrelate(left)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+
+ /** Configuration for rule that prunes a correlate if one of its inputs is empty. */
+ public interface CorrelateEmptyRuleConfig extends PruneEmptyRule.Config {
Review Comment:
If split in separate classes we would have to duplicate the following fragment:
```
@Override
default PruneEmptyRule toRule() {
return new PruneEmptyRule(this) {
@Override
public void onMatch(RelOptRuleCall call) {
if (Bug.CALCITE_5669_FIXED) {
throw new IllegalStateException(
"Class is redundant after fix is merged into Calcite");
}
final Correlate corr = call.rel(0);
final RelNode left = call.rel(1);
final RelNode right = call.rel(2);
final RelNode newRel;
RelBuilder b = call.builder();
```
Don't have a strong opinion on this; let me know what you prefer and will do the refactoring.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
+ int padding = resultType.getFieldCount() - input.getRowType().getFieldCount();
+ List<RexNode> nullLiterals = Collections.nCopies(padding, builder.literal(null));
+ builder.push(input);
+ if (leftPadding) {
+ builder.project(concat(nullLiterals, builder.fields()));
+ } else {
+ builder.project(concat(builder.fields(), nullLiterals));
+ }
+ return builder.convert(resultType, true).build();
+ }
+
+ public static final RelOptRule CORRELATE_RIGHT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(RelNode.class).anyInputs(),
+ b2 -> b2.operand(Values.class).predicate(Values::isEmpty).noInputs()))
+ .withDescription("PruneEmptyCorrelate(right)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+ public static final RelOptRule CORRELATE_LEFT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(Values.class).predicate(Values::isEmpty).noInputs(),
+ b2 -> b2.operand(RelNode.class).anyInputs()))
+ .withDescription("PruneEmptyCorrelate(left)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+
+ /** Configuration for rule that prunes a correlate if one of its inputs is empty. */
+ public interface CorrelateEmptyRuleConfig extends PruneEmptyRule.Config {
Review Comment:
If split in separate classes we would have to duplicate the following fragment:
```java
@Override
default PruneEmptyRule toRule() {
return new PruneEmptyRule(this) {
@Override
public void onMatch(RelOptRuleCall call) {
if (Bug.CALCITE_5669_FIXED) {
throw new IllegalStateException(
"Class is redundant after fix is merged into Calcite");
}
final Correlate corr = call.rel(0);
final RelNode left = call.rel(1);
final RelNode right = call.rel(2);
final RelNode newRel;
RelBuilder b = call.builder();
```
Don't have a strong opinion on this; let me know what you prefer and will do the refactoring.
--
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
[GitHub] [hive] kasakrisz commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1521639063
@zabetak
Does any follow-up required once [CALCITE-5568](https://issues.apache.org/jira/browse/CALCITE-5568) gets resolved?
In the mean time you can merge this patch.
--
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
[GitHub] [hive] sonarcloud[bot] commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1519149555
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4253)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [hive] sonarcloud[bot] commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1519725261
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4253)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [hive] sonarcloud[bot] commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1516872206
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4253)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [hive] sonarcloud[bot] commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1522402023
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4253)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4253&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4253&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4253&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [hive] zabetak commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1521522423
> This is a good contribution to Calcite and the patch looks good to me.
> But normally we should not have LogicalCorrelate after the decorrelation step in Hive. Did you check why it is failed in case of the query mentioned in the jira?
The decorrelator does not cope well with values. Initially, it will return null when it encounters the `LogicalValues` (https://github.com/apache/hive/blob/59058c65457fb7ab9d8575a555034e6633962661/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java#L471) and then it will bail out when treating the `LogicalCorrelate` (https://github.com/apache/hive/blob/59058c65457fb7ab9d8575a555034e6633962661/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java#L1247) since one of the inputs is not rewritten. The problem is still there in recent Calcite (CALCITE-5568).
We can/could treat the problem in the decorrelator in two ways:
a) By changing the `decorrelateRel` method(s); for sure the one for `decorrelateRel(LogicalCorrelate)`
b) Putting the new rules and some more (reductions) inside `HiveRelDecorrelator#decorrelate(root)` (https://github.com/apache/hive/blob/59058c65457fb7ab9d8575a555034e6633962661/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java#L238).
@kasakrisz Let me know if you prefer that we explore one of the alternative paths mentioned above now or if we could postpone for a follow-up.
--
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
[GitHub] [hive] zabetak commented on a diff in pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4253:
URL: https://github.com/apache/hive/pull/4253#discussion_r1176658557
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
+ int padding = resultType.getFieldCount() - input.getRowType().getFieldCount();
+ List<RexNode> nullLiterals = Collections.nCopies(padding, builder.literal(null));
+ builder.push(input);
+ if (leftPadding) {
+ builder.project(concat(nullLiterals, builder.fields()));
+ } else {
+ builder.project(concat(builder.fields(), nullLiterals));
+ }
+ return builder.convert(resultType, true).build();
+ }
+
+ public static final RelOptRule CORRELATE_RIGHT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(RelNode.class).anyInputs(),
+ b2 -> b2.operand(Values.class).predicate(Values::isEmpty).noInputs()))
+ .withDescription("PruneEmptyCorrelate(right)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+ public static final RelOptRule CORRELATE_LEFT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(Values.class).predicate(Values::isEmpty).noInputs(),
+ b2 -> b2.operand(RelNode.class).anyInputs()))
+ .withDescription("PruneEmptyCorrelate(left)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+
+ /** Configuration for rule that prunes a correlate if one of its inputs is empty. */
+ public interface CorrelateEmptyRuleConfig extends PruneEmptyRule.Config {
Review Comment:
The proposed refactoring makes sense and the code is indeed more readable. Fixed in https://github.com/apache/hive/pull/4253/commits/7cf9e827d7b06b15157825ca65ca40486338841f
--
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
[GitHub] [hive] kasakrisz commented on a diff in pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4253:
URL: https://github.com/apache/hive/pull/4253#discussion_r1176385809
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
+ int padding = resultType.getFieldCount() - input.getRowType().getFieldCount();
+ List<RexNode> nullLiterals = Collections.nCopies(padding, builder.literal(null));
+ builder.push(input);
+ if (leftPadding) {
+ builder.project(concat(nullLiterals, builder.fields()));
+ } else {
+ builder.project(concat(builder.fields(), nullLiterals));
+ }
+ return builder.convert(resultType, true).build();
+ }
+
+ public static final RelOptRule CORRELATE_RIGHT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(RelNode.class).anyInputs(),
+ b2 -> b2.operand(Values.class).predicate(Values::isEmpty).noInputs()))
+ .withDescription("PruneEmptyCorrelate(right)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+ public static final RelOptRule CORRELATE_LEFT_INSTANCE = RelRule.Config.EMPTY
+ .withOperandSupplier(b0 ->
+ b0.operand(Correlate.class).inputs(
+ b1 -> b1.operand(Values.class).predicate(Values::isEmpty).noInputs(),
+ b2 -> b2.operand(RelNode.class).anyInputs()))
+ .withDescription("PruneEmptyCorrelate(left)")
+ .withRelBuilderFactory(HiveRelFactories.HIVE_BUILDER)
+ .as(CorrelateEmptyRuleConfig.class)
+ .toRule();
+
+ /** Configuration for rule that prunes a correlate if one of its inputs is empty. */
+ public interface CorrelateEmptyRuleConfig extends PruneEmptyRule.Config {
Review Comment:
When the `Values` is on the left side you only need the `Correlate`. The also `if` goes away at Calcite upgrade.
```
onMatch(RelOptRuleCall call) {
final Correlate corr = call.rel(0);
RelBuilder b = call.builder();
call.transformTo(b.push(corr).empty().build());
}
```
I leave this up to you. :)
--
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
[GitHub] [hive] zabetak commented on pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on PR #4253:
URL: https://github.com/apache/hive/pull/4253#issuecomment-1522002557
> Does any follow-up required once [CALCITE-5568](https://issues.apache.org/jira/browse/CALCITE-5568) gets resolved?
Probably will need to apply the respective changes in `HiveRelDecorrelator`. Logged https://issues.apache.org/jira/browse/HIVE-27296 for tracking this down.
--
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
[GitHub] [hive] zabetak commented on a diff in pull request #4253: HIVE-27278: Simplify correlated queries with empty inputs
Posted by "zabetak (via GitHub)" <gi...@apache.org>.
zabetak commented on code in PR #4253:
URL: https://github.com/apache/hive/pull/4253#discussion_r1176298845
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -192,6 +178,79 @@ public interface JoinRightEmptyRuleConfig extends PruneEmptyRule.Config {
}
}
+ private static RelNode padWithNulls(RelBuilder builder, RelNode input, RelDataType resultType,
+ boolean leftPadding) {
Review Comment:
This will lead to duplicating some code that I would like to avoid hence this refactoring.
--
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