You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Paul Rogers (Code Review)" <ge...@cloudera.org> on 2018/11/06 22:11:03 UTC

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11890


Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................

IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

Impala provides a single test class ExprRewriteRulesTest for all rewrite
rules. Each rule class has an associated test function. Th
SimplifyConditionalRule class is complex, and the result test function
is quite large.

When doing work to modify a particular conditional rewrite, it became
clear that debugging would be much easier if each detailed rewrite had
its own test rather than using one huge test function. This ticket asks
to break up the big function. (JUnit does not care about small vs. large
functions.)

This patch splits the function separate from later work that will modify
the tests themselves.

Also unified test function naming, using the standard "javaStyle" rather
than the unusual "CppStyle" that was used.

Added a few new tests, including some that do not pass (commented out)
to motivate the need for the refactoring.

Testing: this is a test, ran the test to ensure it still passes.

Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
---
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
1 file changed, 112 insertions(+), 24 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 03:29:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 4:

Oh wait, I just saw the thread dump option and I can see the following. It seems to be waiting for new executors. Wondering if it has something to do with the maintenance email from Lars.

Thread #6
	at DSL.parallel(Native Method)
	at WorkflowScript.run(WorkflowScript:8)
	at DSL.timeout(killer task reported done)
	at WorkflowScript.run(WorkflowScript:6)
	at DSL.ansiColor(Native Method)
	at WorkflowScript.run(WorkflowScript:2)
	at DSL.timestamps(Native Method)
	at WorkflowScript.run(WorkflowScript:1)
Thread #13
	at DSL.build(waiting to schedule ubuntu-16.04-from-scratch; blocked: Waiting for next available executor on ‘ubuntu-16.04’)
	at WorkflowScript.run(WorkflowScript:18)
Thread #14
	at DSL.build(waiting to schedule all-build-options-ub1604; blocked: Waiting for next available executor on ‘ubuntu-16.04’)
	at WorkflowScript.run(WorkflowScript:33)
Thread #16
	at DSL.build(waiting to schedule clang-tidy-ub1604; blocked: Waiting for next available executor on ‘ubuntu-16.04’)
	at WorkflowScript.run(WorkflowScript:50)


I could also confirm that from https://jenkins.impala.io/job/parallel-all-tests/4671/flowGraphTable/  For ex: https://jenkins.impala.io/job/parallel-all-tests/4671/execution/node/20/log/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 06 Dec 2018 20:04:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11890/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11890/1//COMMIT_MSG@10
PS1, Line 10: Th
?


http://gerrit.cloudera.org:8080/#/c/11890/1//COMMIT_MSG@26
PS1, Line 26: Added a few new tests, including some that do not pass (commented out)
            : to motivate the need for the refactoring.
Should we add these tests separately along with the jira fixes? Not sure what is the value in including them with this commit.


http://gerrit.cloudera.org:8080/#/c/11890/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

http://gerrit.cloudera.org:8080/#/c/11890/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@438
PS1, Line 438: when true then 30 else 40 end",
             :         rule,
             :         "CASE WHEN id = 1 THEN 10 ELSE 30 END");
nit: format to fewer lines


http://gerrit.cloudera.org:8080/#/c/11890/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@484
PS1, Line 484: testImpala5125
nit: more meaningful name?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 06:02:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 2:

Bharath, anything more I should do on this one?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Mon, 26 Nov 2018 18:55:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1299/ : 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/11890
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 22:57:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................

IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

Impala provides a single test class ExprRewriteRulesTest for all rewrite
rules. Each rule class has an associated test function. The
SimplifyConditionalRule class is complex, and the result test function
is quite large.

When doing work to modify a particular conditional rewrite, it became
clear that debugging would be much easier if each detailed rewrite had
its own test rather than using one huge test function. This ticket asks
to break up the big function. (JUnit does not care about small vs. large
functions.)

This patch splits the function separate from later work that will modify
the tests themselves.

Also unified test function naming, using the standard "javaStyle" rather
than the unusual "CppStyle" that was used.

Testing: this is a test, ran the test to ensure it still passes.

Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Reviewed-on: http://gerrit.cloudera.org:8080/11890
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
---
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
1 file changed, 76 insertions(+), 27 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Bharath Vissapragada: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................

IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

Impala provides a single test class ExprRewriteRulesTest for all rewrite
rules. Each rule class has an associated test function. The
SimplifyConditionalRule class is complex, and the result test function
is quite large.

When doing work to modify a particular conditional rewrite, it became
clear that debugging would be much easier if each detailed rewrite had
its own test rather than using one huge test function. This ticket asks
to break up the big function. (JUnit does not care about small vs. large
functions.)

This patch splits the function separate from later work that will modify
the tests themselves.

Also unified test function naming, using the standard "javaStyle" rather
than the unusual "CppStyle" that was used.

Testing: this is a test, ran the test to ensure it still passes.

Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
---
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
1 file changed, 76 insertions(+), 27 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 3:

Gabor, it is not clear from the jira if the crash is deterministic. Should we wait for it to be fixed or can we trigger another run?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:06:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 3:

Bharath, that issue is not deterministic and I'm currently working on a fix. I'll kick off another GVO on this one.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 06 Dec 2018 07:49:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 4:

The build seems to be hung for 11hrs.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 06 Dec 2018 19:54:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 3:

I think this verify job failed due to this issue:
https://issues.apache.org/jira/browse/IMPALA-7903


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 13:45:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 07 Dec 2018 03:52:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 3:

This patch touches just one FE unit test file. The failure is in the BE web server. Seems a reasonable conclusion that the C++ web server failure is not related to a Java unit test change.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:06:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 06 Dec 2018 23:35:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 06 Dec 2018 07:48:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Tue, 04 Dec 2018 23:11:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 4:

I did a quick pass on the child builds and everything seems to have finished. It is unclear where it is hung. I'm doing force kill for now (unless someone knows if we should collect any diagnostics before we do that).

Pipeline] [AllTests] build (Building ubuntu-16.04-from-scratch)
07:48:25 [AllTests] Scheduling project: ubuntu-16.04-from-scratch
[Pipeline] [UBSAN] build (Building ubuntu-16.04-from-scratch)
07:48:25 [UBSAN] Scheduling project: ubuntu-16.04-from-scratch
[Pipeline] [BuildOptions] build (Building all-build-options-ub1604)
07:48:25 [BuildOptions] Scheduling project: all-build-options-ub1604
[Pipeline] [Python26Compatibility] build (Building python26-incompatibility-check)
07:48:25 [Python26Compatibility] Scheduling project: python26-incompatibility-check
[Pipeline] [TidyAndBuildOnlyAndRat] build (Building clang-tidy-ub1604)
07:48:25 [TidyAndBuildOnlyAndRat] Scheduling project: clang-tidy-ub1604
07:50:53 [Python26Compatibility] Starting building: python26-incompatibility-check #1057
[Pipeline] [Python26Compatibility] }
07:53:00 [AllTests] Starting building: ubuntu-16.04-from-scratch #3793
[Pipeline] [AllTests] }
17:48:25 Cancelling nested steps due to timeout
17:49:25 Body did not finish within grace period; terminating with extreme prejudice


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 06 Dec 2018 19:59:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 07 Dec 2018 03:27:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

Lgtm

http://gerrit.cloudera.org:8080/#/c/11890/2/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

http://gerrit.cloudera.org:8080/#/c/11890/2/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@342
PS2, Line 342: // TODO: IMPALA-7769
             :       //RewritesOk(f + "(1 + 1, id)", rule, "2");
             :       //RewritesOk(f + "(NULL + 1, id)", rule, "id");
             :       //RewritesOk(f + "(cast(null as int), id)", rule, "id");
Remove and add them while fixing 7769?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 19:05:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 2:

(4 comments)

Thanks for the reviews. Addressed the comments, including removing the new (but disabled) tests. Will include those later along with the fixes that they motivated.

http://gerrit.cloudera.org:8080/#/c/11890/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11890/1//COMMIT_MSG@10
PS1, Line 10: Th
> ?
Done


http://gerrit.cloudera.org:8080/#/c/11890/1//COMMIT_MSG@26
PS1, Line 26: Testing: this is a test, ran the test to ensure it still passes.
            : 
> Should we add these tests separately along with the jira fixes? Not sure wh
Sure.


http://gerrit.cloudera.org:8080/#/c/11890/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

http://gerrit.cloudera.org:8080/#/c/11890/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@438
PS1, Line 438: l FALSE, return implicit NULL ELSE.
             :     RewritesOk("decode(1, 1 + 1, id, 1 + 2, 3)", rules, "NULL");
             :     // Multiple TRUE, first one becomes ELSE.
> nit: format to fewer lines
Done


http://gerrit.cloudera.org:8080/#/c/11890/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@484
PS1, Line 484: ("coalesce(1, 
> nit: more meaningful name?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 23:41:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

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

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1377/ : 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/11890
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 16 Nov 2018 00:17:28 +0000
Gerrit-HasComments: No