[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. IMPALA-8271: Refactor the use of Thrift enums for query options This patch refactors the use of Thrift enums with GetThriftEnum helper function that can automatically validate and convert the query option value into the corresponding Thrift enum value. The validation error message has also been improved to list all possible valid query option values. Testing: - Added missing test cases in both BE and E2E - Ran query-options-test.cc - Ran metadata/test_set.py - Ran query_test/test_nested_types.py - Ran query_test/test_scanners.py Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Reviewed-on: http://gerrit.cloudera.org:8080/12682 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/parquet-ambiguous-list-modern.test M testdata/workloads/functional-query/queries/QueryTest/parquet-resolution-by-name.test M testdata/workloads/functional-query/queries/QueryTest/set.test 5 files changed, 125 insertions(+), 160 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 14 Mar 2019 22:43:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 14 Mar 2019 18:36:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3909/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 14 Mar 2019 18:36:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 6: Thanks for cleaning this up btw, I appreciate it when people take the time to make parts of the code more pleasant to work with. -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 14 Mar 2019 18:31:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 6: Code-Review+2 I'll promote to a +2 based on Csaba's +1 and a quick once-over. -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 14 Mar 2019 18:24:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2397/ : 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/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 08 Mar 2019 19:34:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 6: Code-Review+1 (1 comment) Carry Csaba's +1. Tim, can you take a look at it for the +2? http://gerrit.cloudera.org:8080/#/c/12682/5/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12682/5/be/src/service/query-options.cc@504 PS5, Line 504: query_options->__ > nit: could fit in one less lines Done -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 08 Mar 2019 19:02:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. IMPALA-8271: Refactor the use of Thrift enums for query options This patch refactors the use of Thrift enums with GetThriftEnum helper function that can automatically validate and convert the query option value into the corresponding Thrift enum value. The validation error message has also been improved to list all possible valid query option values. Testing: - Added missing test cases in both BE and E2E - Ran query-options-test.cc - Ran metadata/test_set.py - Ran query_test/test_nested_types.py - Ran query_test/test_scanners.py Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/parquet-ambiguous-list-modern.test M testdata/workloads/functional-query/queries/QueryTest/parquet-resolution-by-name.test M testdata/workloads/functional-query/queries/QueryTest/set.test 5 files changed, 125 insertions(+), 160 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/12682/6 -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/12682/5/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12682/5/be/src/service/query-options.cc@504 PS5, Line 504: _type)); nit: could fit in one less lines -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 08 Mar 2019 18:54:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2395/ : 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/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 08 Mar 2019 18:32:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/12682 )
Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options
..
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@343
PS2, Line 343: {0, "CACHE_LOCAL"},
> I see. There are some other query options where not all values are actually
Done. We can actually just pass our own map in GetThriftEnum for the list of
valid enum values.
http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@365
PS2, Line 365:
> Specifying the template does not seem necessary here, as it can be deduced
Done
--
To view, visit http://gerrit.cloudera.org:8080/12682
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641
Gerrit-Change-Number: 12682
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 08 Mar 2019 18:10:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. IMPALA-8271: Refactor the use of Thrift enums for query options This patch refactors the use of Thrift enums with GetThriftEnum helper function that can automatically validate and convert the query option value into the corresponding Thrift enum value. The validation error message has also been improved to list all possible valid query option values. Testing: - Added missing test cases in both BE and E2E - Ran query-options-test.cc - Ran metadata/test_set.py - Ran query_test/test_nested_types.py - Ran query_test/test_scanners.py Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/parquet-ambiguous-list-modern.test M testdata/workloads/functional-query/queries/QueryTest/parquet-resolution-by-name.test M testdata/workloads/functional-query/queries/QueryTest/set.test 5 files changed, 126 insertions(+), 160 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/12682/5 -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@343 PS2, Line 343: uery_options->__set_replica_preference(T > Yeah this is an odd one because we don't remove the unused options, .i.e. C I see. There are some other query options where not all values are actually supported, e.g. I am sure that parquet_compression_codec will only work with a subset of codecs. default_file_format is also questionable, as Impala can create all kinds of tables, but cannot insert into some of them, so I am not sure if it makes sense to allow those as defaults. I am ok with the current state of the patch, but it may make sense to filter unsupported enums, e.g. by adding an optional argument to GetThriftEnum with the list of valid values. http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@365 PS2, Line 365: Specifying the template does not seem necessary here, as it can be deduced from argument 'enum_type'. -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 08 Mar 2019 17:17:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2386/ : 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/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 07 Mar 2019 19:27:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@341 PS2, Line 341: ImpalaQueryOptions::REPLICA_PREFERENCE: > optional: if you are already cleaning things up, these could be replaced wi Done http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@343 PS2, Line 343: uery_options->__set_replica_preference(T > Was this query option left out intentionally? Yeah this is an odd one because we don't remove the unused options, .i.e. CACHE_RACK(1) and DISK_RACK(3) but using those options will throw an error. To keep the existing behavior, I leave this option out. -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 07 Mar 2019 18:48:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. IMPALA-8271: Refactor the use of Thrift enums for query options This patch refactors the use of Thrift enums with GetThriftEnum helper function that can automatically validate and convert the query option value into the corresponding Thrift enum value. The validation error message has also been improved to list all possible valid query option values. Testing: - Added missing test cases in both BE and E2E - Ran query-options-test.cc - Ran metadata/test_set.py - Ran query_test/test_nested_types.py - Ran query_test/test_scanners.py Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/parquet-ambiguous-list-modern.test M testdata/workloads/functional-query/queries/QueryTest/parquet-resolution-by-name.test M testdata/workloads/functional-query/queries/QueryTest/set.test 5 files changed, 110 insertions(+), 147 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/12682/3 -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@341 PS2, Line 341: iequals(value, "true") || iequals(value, "1") optional: if you are already cleaning things up, these could be replaced with a IsTrue(value) after creating an IsTrue() function. http://gerrit.cloudera.org:8080/#/c/12682/2/be/src/service/query-options.cc@343 PS2, Line 343: TImpalaQueryOptions::REPLICA_PREFERENCE: Was this query option left out intentionally? -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 07 Mar 2019 17:38:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 2: I don't have time to fully review this but a quick skim looks good - thanks for making this part of the code more sane! -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Mar 2019 21:02:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12682 ) Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2377/ : 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/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 06 Mar 2019 20:37:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8271: Refactor the use of Thrift enums for query options
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12682 Change subject: IMPALA-8271: Refactor the use of Thrift enums for query options .. IMPALA-8271: Refactor the use of Thrift enums for query options This patch refactors the use of Thrift enums with GetThriftEnum helper function that can automatically validate and convert the query option value into the corresponding Thrift enum value. The validation error message has also been improved to list all possible valid query option values. Testing: - Added missing test cases in both BE and E2E - Ran query-options-test.cc - Ran metadata/test_set.py - Ran query_test/test_nested_types.py - Ran query_test/test_scanners.py Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 --- M be/src/service/query-options-test.cc M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/parquet-ambiguous-list-modern.test M testdata/workloads/functional-query/queries/QueryTest/parquet-resolution-by-name.test M testdata/workloads/functional-query/queries/QueryTest/set.test 5 files changed, 85 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/12682/2 -- To view, visit http://gerrit.cloudera.org:8080/12682 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6d747aae2c689765be72e117ce030ce4e3ce4641 Gerrit-Change-Number: 12682 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya
