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 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/9062/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/9062/4/be/src/exprs/expr-test.cc@5017 PS4, Line 5017: TestValue("cast(-12345.345 as double) % cast(7 as double)", > nit: could be on same line Unfortunately it does not fit on one line. http://gerrit.cloudera.org:8080/#/c/9062/4/be/src/exprs/expr-test.cc@7618 PS4, Line 7618: TEST_F(ExprTest, DecimalOverflowCasts) { > These will only be run with V2, but let's also run them in V1. I think we s Done http://gerrit.cloudera.org:8080/#/c/9062/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java: http://gerrit.cloudera.org:8080/#/c/9062/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2276 PS4, Line 2276: ScalarType.createDecimalType(15, 5), ScalarType.createDecimalType(16, 5)); > Although we have agreed upon these rules, I must still register my objectio Haha, yes, I know what you mean :) http://gerrit.cloudera.org:8080/#/c/9062/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2345 PS4, Line 2345: ScalarType.createDecimalType(15, 5), ScalarType.createDecimalType(16, 5)); > Similarly. Yep http://gerrit.cloudera.org:8080/#/c/9062/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: http://gerrit.cloudera.org:8080/#/c/9062/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@a75 PS4, Line 75: > ??!! did this even run before? Not sure.. probably.. If it didn't run I assume we'd be getting an error because of expected results don't match the actual results http://gerrit.cloudera.org:8080/#/c/9062/4/tests/query_test/test_decimal_casting.py File tests/query_test/test_decimal_casting.py: http://gerrit.cloudera.org:8080/#/c/9062/4/tests/query_test/test_decimal_casting.py@84 PS4, Line 84: if cast_from == 'string': > Should we have a comment that this is deprecating decimal_v1 behavior testi We're deprecating decimal_v1 testing in many places, not just here. I don't think a comment about this is necessary here (unless you guys disagree). -- 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: 4 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 09:01:39 +0000 Gerrit-HasComments: Yes
