Taras Bobrovytsky 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));
> Yes, same with below.  We should clean this up and make an RAII class WithE
Done


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
The test logic did not change as a result of this patch. We just change the 
table and column here to avoid the decimal cast error. Everything else is the 
same in this test.


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
Correct


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 conflic
Done



--
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 20:12:41 +0000
Gerrit-HasComments: Yes

Reply via email to