You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2024/09/09 23:28:17 UTC
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21775
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
IMPALA-13372: Declare batch_size dimension for count_star tests
test_parquet_count_star_optimization, test_kudu_count_star_optimization,
and test_orc_count_star_optimization run their test file twice but
change batch_size option in between. This patch declare batch_size test
dimension and and let vector permutation run the test twice for
different batch_size option. Test class renamed from
TestAggregationQueriesRunOnce to TestAggregationQueriesSingleFormat.
Unused vector argument in test_sampled_ndv and unique_database argument
in test_min_multiple_distinct are dropped. cluster_sizes parameter in
ImpalaTestSuite.add_test_dimensions() and
ImpalaTestSuite.__create_exec_option_dimension() added by IMPALA-12383
are droped and replaced by declaration using
create_exec_option_dimension(). Addressed flake8 issues as well.
Testing:
- Pass test_aggregation.py with core exploration.
Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
---
M tests/common/impala_test_suite.py
M tests/query_test/test_aggregation.py
2 files changed, 86 insertions(+), 67 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/21775/1
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16910/ : 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/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 23:57:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 21:58:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@12
PS1, Line 12: dimension and let vector permutation run the test twice for different
> typo: and and
Done
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@17
PS1, Line 17: in test_min_multiple_distinct are dropped. cluster_sizes parameter in
> The cluster_sizes parameter was meant to be something tests could increment
It is for consistency reason. I see that that exercising ALL_CLUSTER_SIZES is achievable through create_exec_option_dimension().
Today, only TestAggregation use cluster_sizes parameter from ImpalaTestSuite.add_test_dimensions(). Other 17 py.test files declare num_nodes option via cluster_sizes arg of create_exec_option_dimension().
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@20
PS1, Line 20: are dropped and replaced by declaration using
> typo: droped
Done
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 00:35:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@17
PS1, Line 17: in test_min_multiple_distinct are dropped. cluster_sizes parameter in
> It is for consistency reason. I see that that exercising ALL_CLUSTER_SIZES
Ack
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 16:54:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 12:26:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21775/2/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:
http://gerrit.cloudera.org:8080/#/c/21775/2/tests/query_test/test_aggregation.py@360
PS2, Line 360: default batch_size,
: and batch_size=1.
would it be better to say, batch_size=0 and 1? Or mention that default batch_size is 0?
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 17:10:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21775
to look at the new patch set (#2).
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
IMPALA-13372: Declare batch_size dimension for count_star tests
test_parquet_count_star_optimization, test_kudu_count_star_optimization,
and test_orc_count_star_optimization run their test file twice but
change batch_size option in between. This patch declare batch_size test
dimension and let vector permutation run the test twice for different
batch_size option. Test class renamed from TestAggregationQueriesRunOnce
to TestAggregationQueriesSingleFormat.
Unused vector argument in test_sampled_ndv and unique_database argument
in test_min_multiple_distinct are dropped. cluster_sizes parameter in
ImpalaTestSuite.add_test_dimensions() and
ImpalaTestSuite.__create_exec_option_dimension() added by IMPALA-12383
are dropped and replaced by declaration using
create_exec_option_dimension(). Addressed flake8 issues as well.
Testing:
- Pass test_aggregation.py with core exploration.
Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
---
M tests/common/impala_test_suite.py
M tests/query_test/test_aggregation.py
2 files changed, 88 insertions(+), 67 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/21775/2
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10940/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 16:54:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@12
PS1, Line 12: dimension and and let vector permutation run the test twice for
typo: and and
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@17
PS1, Line 17: in test_min_multiple_distinct are dropped. cluster_sizes parameter in
The cluster_sizes parameter was meant to be something tests could incrementally opt into as we expanded tests to cover ALL_CLUSTER_SIZES. I'm not sure I understand the rationale for removing it.
http://gerrit.cloudera.org:8080/#/c/21775/1//COMMIT_MSG@20
PS1, Line 20: are droped and replaced by declaration using
typo: droped
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Sep 2024 23:48:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16911/ : 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/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 01:06:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
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/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
IMPALA-13372: Declare batch_size dimension for count_star tests
test_parquet_count_star_optimization, test_kudu_count_star_optimization,
and test_orc_count_star_optimization run their test file twice but
change batch_size option in between. This patch declare batch_size test
dimension and let vector permutation run the test twice for different
batch_size option. Test class renamed from TestAggregationQueriesRunOnce
to TestAggregationQueriesSingleFormat.
Unused vector argument in test_sampled_ndv and unique_database argument
in test_min_multiple_distinct are dropped. cluster_sizes parameter in
ImpalaTestSuite.add_test_dimensions() and
ImpalaTestSuite.__create_exec_option_dimension() added by IMPALA-12383
are dropped and replaced by declaration using
create_exec_option_dimension(). Addressed flake8 issues as well.
Testing:
- Pass test_aggregation.py with core exploration.
Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Reviewed-on: http://gerrit.cloudera.org:8080/21775
Reviewed-by: Daniel Becker <da...@cloudera.com>
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/common/impala_test_suite.py
M tests/query_test/test_aggregation.py
2 files changed, 88 insertions(+), 67 deletions(-)
Approvals:
Daniel Becker: Looks good to me, but someone else must approve
Michael Smith: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-13372: Declare batch size dimension for count star tests
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21775 )
Change subject: IMPALA-13372: Declare batch_size dimension for count_star tests
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21775/2/tests/query_test/test_aggregation.py
File tests/query_test/test_aggregation.py:
http://gerrit.cloudera.org:8080/#/c/21775/2/tests/query_test/test_aggregation.py@360
PS2, Line 360: default batch_size,
: and batch_size=1.
> would it be better to say, batch_size=0 and 1? Or mention that default bat
Yes, default batch_size=0. While batch_size=1 is really just passing 1 rows at a time between plan node and fragment. I'd like to keep it this way, since precommit job already run.
--
To view, visit http://gerrit.cloudera.org:8080/21775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b0399ee3b39a5b0bbfdf8a05680ab1fb23d8f87
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Sep 2024 17:26:30 +0000
Gerrit-HasComments: Yes