You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org> on 2020/02/24 21:32:39 UTC

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15285


Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................

IMPALA-4080 [part 4]: Moved code responsible for codegen from select
and union's exec node to their plan node.

Refactored code responsible for codegen from select and union's exec
node to their plan node.

Testing:
Successfully ran exhaustive tests.
Manually verified that codegen works for all modified exec nodes.

Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
---
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M be/src/exec/union-node.cc
M be/src/exec/union-node.h
4 files changed, 75 insertions(+), 37 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 18:23:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 18:23:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15285 )

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................

IMPALA-4080 [part 4]: Moved code responsible for codegen from select
and union's exec node to their plan node.

Refactored code responsible for codegen from select and union's exec
node to their plan node.

Testing:
Successfully ran exhaustive tests.
Manually verified that codegen works for all modified exec nodes.

Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Reviewed-on: http://gerrit.cloudera.org:8080/15285
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M be/src/exec/union-node.cc
M be/src/exec/union-node.h
4 files changed, 76 insertions(+), 37 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 4
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Feb 2020 21:54:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 2: Code-Review+2

Carrying over Tim's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 18:23:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 22:51:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................

IMPALA-4080 [part 4]: Moved code responsible for codegen from select
and union's exec node to their plan node.

Refactored code responsible for codegen from select and union's exec
node to their plan node.

Testing:
Successfully ran exhaustive tests.
Manually verified that codegen works for all modified exec nodes.

Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
---
M be/src/exec/select-node.cc
M be/src/exec/select-node.h
M be/src/exec/union-node.cc
M be/src/exec/union-node.h
4 files changed, 76 insertions(+), 37 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 19:06:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15285/1/be/src/exec/union-node.h
File be/src/exec/union-node.h:

http://gerrit.cloudera.org:8080/#/c/15285/1/be/src/exec/union-node.h@61
PS1, Line 61:   int first_materialized_child_idx_;
Initialise to an invalid value?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 16:26:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.

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

Change subject: IMPALA-4080 [part 4]: Moved code responsible for codegen from select and union's exec node to their plan node.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15285/1/be/src/exec/union-node.h
File be/src/exec/union-node.h:

http://gerrit.cloudera.org:8080/#/c/15285/1/be/src/exec/union-node.h@61
PS1, Line 61:   int first_materialized_child_idx_ = -1;
> Initialise to an invalid value?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0610b93b9a13da19e35151b231349c0ab16071d2
Gerrit-Change-Number: 15285
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 18:22:53 +0000
Gerrit-HasComments: Yes