Zach Amsden has posted comments on this change. ( http://gerrit.cloudera.org:8080/9062 )
Change subject: IMPALA-4924: Enable Decimal V2 by default ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/9062/5/be/src/exprs/expr-test.cc@7688 PS5, Line 7688: ColumnType::CreateDecimalType(15,2)); > my understanding is we need to executor_->PopExecOption(); at the end of th Yes, same with below. We should clean this up and make an RAII class WithExecOption so this is automatic when leaving scope. (Not in this diff) http://gerrit.cloudera.org:8080/#/c/9062/5/testdata/workloads/functional-query/queries/QueryTest/uda.test File testdata/workloads/functional-query/queries/QueryTest/uda.test: http://gerrit.cloudera.org:8080/#/c/9062/5/testdata/workloads/functional-query/queries/QueryTest/uda.test@85 PS5, Line 85: select agg_decimal_intermediate(cast(c3 as decimal(2,1)), 2), count(*) going on blind faith here http://gerrit.cloudera.org:8080/#/c/9062/5/tests/hs2/test_hs2.py File tests/hs2/test_hs2.py: http://gerrit.cloudera.org:8080/#/c/9062/5/tests/hs2/test_hs2.py@443 PS5, Line 443: assert "Invalid base64 string; input length is 3, which is not a multiple of 4" in log ok, we're just testing get_log here http://gerrit.cloudera.org:8080/#/c/9062/5/tests/query_test/test_decimal_casting.py File tests/query_test/test_decimal_casting.py: http://gerrit.cloudera.org:8080/#/c/9062/5/tests/query_test/test_decimal_casting.py@126 PS5, Line 126: val, precision, vector.get_value('cast_from')).format(val, precision, scale) Unnecessary change. Yes it is nicer, but it could cause more merge conflicts going forward. -- To view, visit http://gerrit.cloudera.org:8080/9062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7 Gerrit-Change-Number: 9062 Gerrit-PatchSet: 5 Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-Comment-Date: Fri, 19 Jan 2018 18:17:46 +0000 Gerrit-HasComments: Yes
