You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/02/28 06:20:19 UTC
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12635
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M tests/metadata/test_compute_stats.py
3 files changed, 35 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/3
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc@235
PS3, Line 235: break;
> I created some macros to simplify a lot of things. Please take a look at it
Thanks! It looks much less noisy to me.
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@175
PS7, Line 175: #define BUILD_QUERY_OPT_FN(ENUM) \
: static bool get##ENUM(const string& value, ENUM::type* enum_value) { \
Most of this could be expressed with a template function that gets the enum type as parameter, and the map as an argument. It wouldn't make things simpler, but template functions are probably less scary than macros.
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@178
PS7, Line 178: value == e.second
The original comparison was case insensitive, which is expected for query options.
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@186
PS7, Line 186: GET_QUERY_OPT
I would prefer to mention "thrift" and "enum" in the name.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 22:23:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12635/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12635/4//COMMIT_MSG@16
PS4, Line 16: - Ran all tests in test_compute.stats.py
Do we have negative tests that inserting with unsupported compression codecs fails?
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 16:34:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 11: Code-Review+2
(2 comments)
Carry Tim's +2.
http://gerrit.cloudera.org:8080/#/c/12635/10/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/10/be/src/service/query-options.cc@180
PS10, Line 180: ss << ", ";
> It feels a little clunky to build the string every time and then throw it a
Done
http://gerrit.cloudera.org:8080/#/c/12635/10/tests/metadata/test_compute_stats.py
File tests/metadata/test_compute_stats.py:
http://gerrit.cloudera.org:8080/#/c/12635/10/tests/metadata/test_compute_stats.py@81
PS10, Line 81: compute
> compute
Done
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Mar 2019 05:22:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2353/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Mar 2019 06:07:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Mar 2019 05:23:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2298/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 16:18:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_compute_stats.py
4 files changed, 44 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/10
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_compute_stats.py
4 files changed, 52 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/8
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@175
PS7, Line 175: template<typename ENUM_TYPE>
: static bool get_thrift_enum(const string& value,
> Most of this could be expressed with a template function that gets the enum
Done
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@178
PS7, Line 178: es) {
> The original comparison was case insensitive, which is expected for query o
Oops. Done.
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@186
PS7, Line 186:
> I would prefer to mention "thrift" and "enum" in the name.
Done
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 16:09:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2300/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 17:26:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc@235
PS3, Line 235: if (iequals(value, "none") || iequals(value, "0")) {
optional: these big else if chains look error prone - it would be probably safer to create a macro or template function that takes 'value' and the VALUES_TO_NAMES map of a thrift enum (_THdfsCompression_VALUES_TO_NAMES in this case), iterates the map, and returns the enum where the name or the number matches.
This would have the side effect that if the .thrift file is updated and a new value is added to the enum, then this new value would be recognized as query option automatically. The list of enum values accepted by Impala could be also added if this is a problem.
http://gerrit.cloudera.org:8080/#/c/12635/3/tests/metadata/test_compute_stats.py
File tests/metadata/test_compute_stats.py:
http://gerrit.cloudera.org:8080/#/c/12635/3/tests/metadata/test_compute_stats.py@79
PS3, Line 79: @pytest.mark.execute_serially
Does this have to be executed serially? It should not collide with other tests because of the unique_database.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 13:42:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc@235
PS3, Line 235: break;
> I think that the current "else if chain" is also O(N), so replacing it with
I created some macros to simplify a lot of things. Please take a look at it again. If it's good, I'll create another JIRA to use the same pattern for move other query options to use the new style.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 20:43:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_compute_stats.py
4 files changed, 44 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/9
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12635/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12635/4//COMMIT_MSG@16
PS4, Line 16: - Ran all tests in test_compute.stats.py
> Do we have negative tests that inserting with unsupported compression codec
Yup: https://github.com/apache/impala/blob/master/testdata/workloads/functional-query/queries/QueryTest/set.test#L94-L97. I updated the git commit message to also run test_set.py.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 16:43:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 10: Code-Review+2
(2 comments)
I have one potential improvement but otherwise LGTM
http://gerrit.cloudera.org:8080/#/c/12635/10/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/10/be/src/service/query-options.cc@180
PS10, Line 180: for (const auto& e : enum_values_to_names) {
It feels a little clunky to build the string every time and then throw it away. I don't think it *really* matters though.
I'd probably factor out the string building into a separate function that returns a comma-separated list of supported enum values and have two simple loops instead of one complex loop.
I'll leave it up to you whether that's better though.
http://gerrit.cloudera.org:8080/#/c/12635/10/tests/metadata/test_compute_stats.py
File tests/metadata/test_compute_stats.py:
http://gerrit.cloudera.org:8080/#/c/12635/10/tests/metadata/test_compute_stats.py@81
PS10, Line 81: compuete
compute
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 18:26:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M tests/metadata/test_compute_stats.py
3 files changed, 34 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/5
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2319/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 18:27:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 12:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3874/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Mar 2019 05:23:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc@235
PS3, Line 235: if (iequals(value, "none") || iequals(value, "0")) {
> I don't know if it's a big issue, but this will make the operation to be li
I think that the current "else if chain" is also O(N), so replacing it with a single pass over VALUES_TO_NAME would not make it worse. I don't think that being O(N) is a big issue if N=10.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 16:44:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
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/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Reviewed-on: http://gerrit.cloudera.org:8080/12635
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_compute_stats.py
4 files changed, 51 insertions(+), 19 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M tests/metadata/test_compute_stats.py
3 files changed, 37 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/7
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2296/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 07:04:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2305/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 21:24:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 10: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@264
PS8, Line 264: query_options->__set_hbase_caching(atoi(value.c_str()));
: break;
: case TImpalaQueryOptions::HBASE_CACHE_BLOCKS:
: query_options->__set_hbase_cache_blocks(
: iequals(value, "true") || iequals(value, "1"));
> Good idea. I can make the whole thing in a single pass.
Thanks, I think it became more readable.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 23:01:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2322/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 22:35:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 9:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@178
PS7, Line 178:
> Can you add a few tests to verify case insensitivity?
Done
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@176
PS8, Line 176: s GetThriftEnum
> Other functions in the class use camel case.
Done
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@188
PS8, Line 188: lue = static_cast<ENUM
> same as line 176
Done
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@264
PS8, Line 264: query_options->__set_hbase_caching(atoi(value.c_str()));
: break;
: case TImpalaQueryOptions::HBASE_CACHE_BLOCKS:
: query_options->__set_hbase_cache_blocks(
: iequals(value, "true") || iequals(value, "1"));
> optional: get_thrift_enum could have a signature similar to ParseMemValue()
Good idea. I can make the whole thing in a single pass.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 17:42:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 8:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@178
PS7, Line 178: es) {
> Oops. Done.
Can you add a few tests to verify case insensitivity?
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@176
PS8, Line 176: get_thrift_enum
Other functions in the class use camel case.
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@188
PS8, Line 188: get_thrift_enum_values
same as line 176
http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@264
PS8, Line 264: if (!get_thrift_enum<THdfsCompression::type>(value,
: _THdfsCompression_VALUES_TO_NAMES, &enum_type)) {
: return Status(Substitute("Invalid compression codec: '$0'. Valid compression "
: "codecs are $1.", value,
: get_thrift_enum_values(_THdfsCompression_VALUES_TO_NAMES)));
optional: get_thrift_enum could have a signature similar to ParseMemValue() to simplify its usage. I think it is ok to always write "Valid values are: ..."
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 17:07:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
- Ran all tests in test_set.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_compute_stats.py
4 files changed, 51 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/11
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 10:
I created a follow-up JIRA: https://issues.apache.org/jira/browse/IMPALA-8271 to fix update the rest of Thrift enums in query-options.cc
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 23:11:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Mar 2019 09:39:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
IMPALA-8254: Fix error when running compute stats with compression_codec set
This patch fixes an error when running compute stats with
compression_codec set. This patch also updates missing compression
codecs from the compression_codec query option.
Testing:
- Updated BE query-options-test
- Added test_compute.stats.py
- Ran all tests in test_compute.stats.py
Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M tests/metadata/test_compute_stats.py
3 files changed, 34 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/12635/4
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2317/ : 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/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 16:52:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8254: Fix error when running compute stats with compression codec set
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12635 )
Change subject: IMPALA-8254: Fix error when running compute stats with compression_codec set
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12635/3/be/src/service/query-options.cc@235
PS3, Line 235: if (iequals(value, "none") || iequals(value, "0")) {
> optional: these big else if chains look error prone - it would be probably
I don't know if it's a big issue, but this will make the operation to be linear O(N) (since we have to iterate the map) vs constant O(1). Unfortunately in the C++ Thrift generated code, I only see _THdfsCompression_VALUES_TO_NAMES and no _THdfsCompression_NAMES_TO_VALUES, which could make the operation constant by doing a two lookups depending on whether the value is string or integer. Let me know what you think?
http://gerrit.cloudera.org:8080/#/c/12635/3/tests/metadata/test_compute_stats.py
File tests/metadata/test_compute_stats.py:
http://gerrit.cloudera.org:8080/#/c/12635/3/tests/metadata/test_compute_stats.py@79
PS3, Line 79: @SkipIfS3.eventually_consiste
> Does this have to be executed serially? It should not collide with other te
Yup, I don't think we need it. Removed.
--
To view, visit http://gerrit.cloudera.org:8080/12635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Feb 2019 15:43:32 +0000
Gerrit-HasComments: Yes