You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2021/02/12 19:58:44 UTC

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17064


Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................

IMPALA-9745: Propagate source type when doing constant propagation

When doing constant propagation the source type was not being
propagated to the target expression leading to an analysis failure.
The behavior is most easily reproducible with STRING to TIMESTAMP
conversion in the presence of other predicates.

This patch fixes this by adding an implicit cast if needed for such
cases.

Testing:
 - Added planner test and ran other planner tests
 - Added end-to-end test

Change-Id: Ic3853478945229440f733c256ea225639f9178ff
---
M fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test
3 files changed, 38 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/17064/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................

IMPALA-9745: Propagate source type when doing constant propagation

When doing constant propagation the source type was not being
propagated to the target expression leading to an analysis failure.
The behavior is most easily reproducible with STRING to TIMESTAMP
conversion in the presence of other predicates.

This patch fixes this by adding an implicit cast if needed for such
cases.

Testing:
 - Added planner test and ran other planner tests
 - Added end-to-end test

Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Reviewed-on: http://gerrit.cloudera.org:8080/17064
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Aman Sinha <am...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test
3 files changed, 38 insertions(+), 2 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved
  Aman Sinha: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 22:33:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 23:34:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8134/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 22:17:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/8132/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 20:09:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 4: Verified+1

> Patch Set 4: Verified-1
> 
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6888/

The failure is due to flaky test https://issues.apache.org/jira/browse/IMPALA-10316.  Hence, I am doing a +1 on Verified.


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Feb 2021 23:02:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Qifan Chen, Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17064

to look at the new patch set (#3).

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................

IMPALA-9745: Propagate source type when doing constant propagation

When doing constant propagation the source type was not being
propagated to the target expression leading to an analysis failure.
The behavior is most easily reproducible with STRING to TIMESTAMP
conversion in the presence of other predicates.

This patch fixes this by adding an implicit cast if needed for such
cases.

Testing:
 - Added planner test and ran other planner tests
 - Added end-to-end test

Change-Id: Ic3853478945229440f733c256ea225639f9178ff
---
M fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test
3 files changed, 38 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/17064/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17064/2/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java
File fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java:

http://gerrit.cloudera.org:8080/#/c/17064/2/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java@186
PS2, Line 186:     Expr expr = source.getType() != target.getType() ?
Should this use !....equals() instead of !=



-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 21:33:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17064/2/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java
File fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java:

http://gerrit.cloudera.org:8080/#/c/17064/2/fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java@186
PS2, Line 186:     Expr expr = source.getType().equals(target.getType()) ?
> Should this use !....equals() instead of !=
Yes.  I also flipped it to use just equals instead of not-equals logic.



-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 21:59:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6888/


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Feb 2021 05:13:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has removed a vote on this change.

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Qifan Chen, Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17064

to look at the new patch set (#2).

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................

IMPALA-9745: Propagate source type when doing constant propagation

When doing constant propagation the source type was not being
propagated to the target expression leading to an analysis failure.
The behavior is most easily reproducible with STRING to TIMESTAMP
conversion in the presence of other predicates.

This patch fixes this by adding an implicit cast if needed for such
cases.

Testing:
 - Added planner test and ran other planner tests
 - Added end-to-end test

Change-Id: Ic3853478945229440f733c256ea225639f9178ff
---
M fe/src/main/java/org/apache/impala/analysis/ConstantPredicateHandler.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/range-constant-propagation.test
3 files changed, 38 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/17064/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6888/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 23:34:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9745: Propagate source type when doing constant propagation

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17064 )

Change subject: IMPALA-9745: Propagate source type when doing constant propagation
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8133/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/17064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3853478945229440f733c256ea225639f9178ff
Gerrit-Change-Number: 17064
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Feb 2021 21:07:26 +0000
Gerrit-HasComments: No