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 2025/01/02 19:55:18 UTC
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22291
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
IMPALA-13636: Fix target file_format in TestTpcdsInsert
TestTpcdsInsert creates a temporary table to test insert functionality.
It has three problems:
1. It does not use unique_database parameter, so the temporary table is
not cleaned up after test finished.
2. It ignores file_format from test vector, causing inconsistency in the
temporary table's file format. str_insert is always in PARQUET format,
while store_sales_insert is always in TEXTFILE format.
3. Insert in TEXTFILE format is never exercised, even in exhaustive
exploration.
This patch fixes all three problems and few flake8 warnings in
test_tpcds_queries.py.
Testing:
- Run bin/run-all-tests.sh with
EE_TEST_FILES="query_test/test_tpcds_queries.py::TestTpcdsInsert"
Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
---
M bin/run-all-tests.sh
M testdata/workloads/tpcds-insert/queries/expr-insert.test
M testdata/workloads/tpcds-insert/queries/partitioned-insert.test
M tests/query_test/test_tpcds_queries.py
4 files changed, 31 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/22291/1
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/17829/ : 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/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2025 20:21:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/17830/ : 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/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2025 20:37:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Jason Fehr, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22291
to look at the new patch set (#3).
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
IMPALA-13636: Fix target file_format in TestTpcdsInsert
TestTpcdsInsert creates a temporary table to test insert functionality.
It has three problems:
1. It does not use unique_database parameter, so the temporary table is
not cleaned up after test finished.
2. It ignores file_format from test vector, causing inconsistency in the
temporary table's file format. str_insert is always in PARQUET format,
while store_sales_insert is always in TEXTFILE format.
3. text file_format dimension is never exercised, because
--workload_exploration_strategy in run-all-tests.sh does not
explicitly list tpcds-insert workload.
This patch fixes all three problems and few flake8 warnings in
test_tpcds_queries.py.
Testing:
- Run bin/run-all-tests.sh with
EXPLORATION_STRATEGY=exhaustive
EE_TEST=true
EE_TEST_FILES="query_test/test_tpcds_queries.py::TestTpcdsInsert"
Verified that the temporary table format follows file_format
dimension.
Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
---
M bin/run-all-tests.sh
M buildall.sh
M testdata/workloads/tpcds-insert/queries/expr-insert.test
M testdata/workloads/tpcds-insert/queries/partitioned-insert.test
M tests/query_test/test_tpcds_queries.py
5 files changed, 32 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/22291/3
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22291/3/tests/query_test/test_tpcds_queries.py
File tests/query_test/test_tpcds_queries.py:
http://gerrit.cloudera.org:8080/#/c/22291/3/tests/query_test/test_tpcds_queries.py@711
PS3, Line 711: return {'$FILE_FORMAT': stored_as}
I wonder if this should be a standard part of ImpalaTestSuite replacements.
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jan 2025 21:32:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22291/3/tests/query_test/test_tpcds_queries.py
File tests/query_test/test_tpcds_queries.py:
http://gerrit.cloudera.org:8080/#/c/22291/3/tests/query_test/test_tpcds_queries.py@711
PS3, Line 711: return {'$FILE_FORMAT': stored_as}
> I wonder if this should be a standard part of ImpalaTestSuite replacements.
We can add it by passing the test vector into ImpalaTestSuite.__do_replacements, but there is not much tests that does INSERT OVERWRITE or CTAS with varying file_format dimension.
$ git grep -n is_supported_insert_format
tests/common/test_dimensions.py:347:def is_supported_insert_format(table_format):
tests/query_test/test_insert.py:36: is_supported_insert_format)
tests/query_test/test_insert.py:409: is_supported_insert_format(v.get_value('table_format')))
tests/query_test/test_tpcds_queries.py:32: is_supported_insert_format)
tests/query_test/test_tpcds_queries.py:705: is_supported_insert_format(v.get_value('table_format')))
The other test in test_insert.py does not use run_test_case() at all.
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jan 2025 23:12:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22291/3/tests/query_test/test_tpcds_queries.py
File tests/query_test/test_tpcds_queries.py:
http://gerrit.cloudera.org:8080/#/c/22291/3/tests/query_test/test_tpcds_queries.py@711
PS3, Line 711: return {'$FILE_FORMAT': stored_as}
> We can add it by passing the test vector into ImpalaTestSuite.__do_replacem
Ack. Not useful yet.
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Jan 2025 23:43:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 3: Verified+1
Carry Verified+1 from the tip of chain.
https://gerrit.cloudera.org/c/22297/2#message-ff695c18_116c7aaa
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jan 2025 22:15:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
IMPALA-13636: Fix target file_format in TestTpcdsInsert
TestTpcdsInsert creates a temporary table to test insert functionality.
It has three problems:
1. It does not use unique_database parameter, so the temporary table is
not cleaned up after test finished.
2. It ignores file_format from test vector, causing inconsistency in the
temporary table's file format. str_insert is always in PARQUET format,
while store_sales_insert is always in TEXTFILE format.
3. text file_format dimension is never exercised, because
--workload_exploration_strategy in run-all-tests.sh does not
explicitly list tpcds-insert workload.
This patch fixes all three problems and few flake8 warnings in
test_tpcds_queries.py.
Testing:
- Run bin/run-all-tests.sh with
EXPLORATION_STRATEGY=exhaustive
EE_TEST=true
EE_TEST_FILES="query_test/test_tpcds_queries.py::TestTpcdsInsert"
Verified that the temporary table format follows file_format
dimension.
Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Reviewed-on: http://gerrit.cloudera.org:8080/22291
Reviewed-by: Michael Smith <mi...@cloudera.com>
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Riza Suminto <ri...@cloudera.com>
---
M bin/run-all-tests.sh
M buildall.sh
M testdata/workloads/tpcds-insert/queries/expr-insert.test
M testdata/workloads/tpcds-insert/queries/partitioned-insert.test
M tests/query_test/test_tpcds_queries.py
5 files changed, 32 insertions(+), 28 deletions(-)
Approvals:
Michael Smith: Looks good to me, but someone else must approve
Csaba Ringhofer: Looks good to me, approved
Riza Suminto: Verified
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Jason Fehr, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22291
to look at the new patch set (#2).
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
IMPALA-13636: Fix target file_format in TestTpcdsInsert
TestTpcdsInsert creates a temporary table to test insert functionality.
It has three problems:
1. It does not use unique_database parameter, so the temporary table is
not cleaned up after test finished.
2. It ignores file_format from test vector, causing inconsistency in the
temporary table's file format. str_insert is always in PARQUET format,
while store_sales_insert is always in TEXTFILE format.
3. text file_format dimension is never exercised, because
--workload_exploration_strategy in run-all-tests.sh does not
explicitly list tpcds-insert workload.
This patch fixes all three problems and few flake8 warnings in
test_tpcds_queries.py.
Testing:
- Run bin/run-all-tests.sh with
EXPLORATION_STRATEGY=exhaustive
EE_TEST=true
EE_TEST_FILES="query_test/test_tpcds_queries.py::TestTpcdsInsert"
Verified that the temporary table format follows file_format
dimension.
Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
---
M bin/run-all-tests.sh
M buildall.sh
M testdata/workloads/tpcds-insert/queries/expr-insert.test
M testdata/workloads/tpcds-insert/queries/partitioned-insert.test
M tests/query_test/test_tpcds_queries.py
5 files changed, 32 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/22291/2
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13636: Fix target file format in TestTpcdsInsert
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22291 )
Change subject: IMPALA-13636: Fix target file_format in TestTpcdsInsert
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea621ec1d6a53eba9558b0daa3a4cc97fbcc67ae
Gerrit-Change-Number: 22291
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Jan 2025 16:04:36 +0000
Gerrit-HasComments: No