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

Reply via email to