[Impala-ASF-CR] IMPALA-3436: Return a decimal when rounding a double

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8398 )

Change subject: IMPALA-3436: Return a decimal when rounding a double
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:

http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@285
PS3, Line 285:   [['round_v1','dround_v1'],
> * Yes, we want to remove decimal_v1.
I agree with Alex. I don't think it makes much sense to have a round() function 
that returns a double.

We can clearly document that round(double, int) requires a constant second 
argument in decimal_v2. If it break someone, they were probably doing something 
weird in the first place.



--
To view, visit http://gerrit.cloudera.org:8080/8398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I240ec70da7996bbfefcf6438eba4dff8d33d7bd6
Gerrit-Change-Number: 8398
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 16 Nov 2017 00:26:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5017: Error on decimal overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8404 )

Change subject: IMPALA-5017: Error on decimal overflow
..

IMPALA-5017: Error on decimal overflow

Before this patch, decimal operations would either silently overflow (in
the case of sum() and avg()), or produce a warning.

In this patch, the behaviour is changed so that an error is produced in
the case of overflow when DECIMAL_v2 is enabled. Decimal v1 behaviour is
unchanged.

We introduce overflow checks when computing sum() and avg(). This
results in a ~30% performance regression when we are in decimal v2 mode
compared to decimal v1.

Benchmarks:
  Query:
  select sum(dec_38_19) from decimal_tbl
Decimal v1: 13.09s
Decimal v2: 17.10s

  Query:
  select avg(dec_38_19) from decimal_tbl
Decimal v1: 13.60s
Decimal v2: 19.10s

  Query:
  select avg(dec_9_5) from decimal_tbl
Decimal v1: 12.06s
Decimal v2: 18.07s

Testing:
- Added several end to end tests.
- Updated Expr tests to check for error in case of overflow.

Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-codegen-test.cc
M be/src/exprs/expr-test.cc
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
6 files changed, 230 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/8404/3
--
To view, visit http://gerrit.cloudera.org:8080/8404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Gerrit-Change-Number: 8404
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-5017: Error on decimal overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/8404 )

Change subject: IMPALA-5017: Error on decimal overflow
..

IMPALA-5017: Error on decimal overflow

Before this patch, decimal operations would either silently overflow (in
the case of sum() and avg()), or produce a warning.

In this patch, the behaviour is changed so that an error is produced in
the case of overflow when DECIMAL_v2 is enabled. Decimal v1 behaviour is
unchanged.

We introduce overflow checks when computing sum() and avg(). This
results in a ~30% performance regression when we are in decimal v2 mode
compared to decimal v1.

Benchmarks:
  Query:
  select sum(dec_38_19) from decimal_tbl
Decimal v1: 11.57s
Decimal v2: 16.58s

  Query:
  select avg(dec_38_19) from decimal_tbl
Decimal v1: 12.08s
Decimal v2: 17.08s

The performance regression is not as bad if we are computing the sum or
average of decimal column with a lower precision:

  Query:
  select sum(dec_9_5) from decimal_tbl
Decimal v1: 11.06s
Decimal v2: 13.08s

  Query:
  select avg(dec_9_5) from decimal_tbl
Decimal v1: 11.56s
Decimal v2: 13.57s

Testing:
- Added several end to end tests.
- Updated Expr tests to check for error in case of overflow.

Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-codegen-test.cc
M be/src/exprs/expr-test.cc
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
6 files changed, 230 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/8404/4
--
To view, visit http://gerrit.cloudera.org:8080/8404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Gerrit-Change-Number: 8404
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-5017: Error on decimal overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8404 )

Change subject: IMPALA-5017: Error on decimal overflow
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8404/3/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8404/3/be/src/exprs/aggregate-functions-ir.cc@415
PS3, Line 415: ctx->SetError("Avg computation overflowed");
> did it help performance?
Yes it did. I updated the benchmark results in the commit message.



--
To view, visit http://gerrit.cloudera.org:8080/8404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Gerrit-Change-Number: 8404
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Thu, 30 Nov 2017 23:09:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5017: Error on decimal overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8404 )

Change subject: IMPALA-5017: Error on decimal overflow
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8404/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8404/4//COMMIT_MSG@43
PS4, Line 43:
> are these numbers all from release build, no debug build?
Yes, these numbers (and all other benchmarks that I've done) are from a release 
build.



--
To view, visit http://gerrit.cloudera.org:8080/8404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Gerrit-Change-Number: 8404
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Thu, 30 Nov 2017 23:16:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5017: Error on decimal overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/8404 )

Change subject: IMPALA-5017: Error on decimal overflow
..

IMPALA-5017: Error on decimal overflow

Before this patch, decimal operations would either silently overflow (in
the case of sum() and avg()), or produce a warning.

In this patch, the behaviour is changed so that an error is produced in
the case of overflow when DECIMAL_v2 is enabled. Decimal v1 behaviour is
unchanged.

We introduce overflow checks when computing sum() and avg(). This
results in a ~30% performance regression when we are in decimal v2 mode
compared to decimal v1.

Benchmarks:
  Query:
  select sum(dec_38_19) from decimal_tbl
Decimal v1: 11.57s
Decimal v2: 16.58s

  Query:
  select avg(dec_38_19) from decimal_tbl
Decimal v1: 12.08s
Decimal v2: 17.08s

The performance regression is not as bad if we are computing the sum or
average of decimal column with a lower precision:

  Query:
  select sum(dec_9_5) from decimal_tbl
Decimal v1: 11.06s
Decimal v2: 13.08s

  Query:
  select avg(dec_9_5) from decimal_tbl
Decimal v1: 11.56s
Decimal v2: 13.57s

Testing:
- Added several end to end tests.
- Updated Expr tests to check for error in case of overflow.

Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-codegen-test.cc
M be/src/exprs/expr-test.cc
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
M tests/hs2/test_hs2.py
7 files changed, 231 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/8404/5
--
To view, visit http://gerrit.cloudera.org:8080/8404
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Gerrit-Change-Number: 8404
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6284: Mark the intermediate decimal avg struct as packed

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8836 )

Change subject: IMPALA-6284: Mark the intermediate decimal avg struct as packed
..

IMPALA-6284: Mark the intermediate decimal avg struct as packed

We saw some failures on the exhaustive release build because the
compiler assumed that the pointer to the intermediate struct that is
used for computing decimal average was aligned.

To fix the problem, we mark the struct with a "packed" attribute so
that the compiler does not expect it to be aligned.

Testing:
- Ran the failing test locally on an release build and it passed.

Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
---
M be/src/exprs/aggregate-functions-ir.cc
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test
3 files changed, 7 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8836/3
--
To view, visit http://gerrit.cloudera.org:8080/8836
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
Gerrit-Change-Number: 8836
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6284: Mark the intermediate decimal avg struct as packed

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8836 )

Change subject: IMPALA-6284: Mark the intermediate decimal avg struct as packed
..


Patch Set 3: Code-Review+2

Fixed the expected size in one of our tests. Forwarding the +2.


--
To view, visit http://gerrit.cloudera.org:8080/8836
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
Gerrit-Change-Number: 8836
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 15 Dec 2017 23:52:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6284: Mark the intermediate decimal avg struct as packed

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8836 )

Change subject: IMPALA-6284: Mark the intermediate decimal avg struct as packed
..


Patch Set 1:

I'm not really sure what a new test for this would look like. We already have 
tests that trigger the problem on an exhaustive release build.


--
To view, visit http://gerrit.cloudera.org:8080/8836
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
Gerrit-Change-Number: 8836
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 14 Dec 2017 17:50:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6284: Mark the intermediate decimal avg struct as packed

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8836 )

Change subject: IMPALA-6284: Mark the intermediate decimal avg struct as packed
..

IMPALA-6284: Mark the intermediate decimal avg struct as packed

We saw some failures on the exhaustive release build because the
compiler assumed that the pointer to the intermediate struct that is
used for computing decimal average was aligned.

To fix the problem, we mark the struct with a "packed" attribute so
that the compiler does not expect it to be aligned.

Testing:
- Ran the failing test locally on an release build and it passed.

Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
---
M be/src/exprs/aggregate-functions-ir.cc
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
2 files changed, 6 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8836/2
--
To view, visit http://gerrit.cloudera.org:8080/8836
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
Gerrit-Change-Number: 8836
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6284: Mark the intermediate decimal avg struct as packed

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8836 )

Change subject: IMPALA-6284: Mark the intermediate decimal avg struct as packed
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8836/1/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8836/1/be/src/exprs/aggregate-functions-ir.cc@388
PS1, Line 388:   DCHECK_EQ(dst->len, sizeof(DecimalAvgState));
> Did you test on a debug build? I think there's a constant in the FE that ne
Fixed. The BE tests on debug build passed after the fix (they were failing 
before).



--
To view, visit http://gerrit.cloudera.org:8080/8836
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
Gerrit-Change-Number: 8836
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 14 Dec 2017 23:00:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8833 )

Change subject: IMPALA-6300: Fix decimal modulo overflow
..

IMPALA-6300: Fix decimal modulo overflow

In order to compute the modulo of two decimals, we need to bring the
underlying datatype to the same scale first. It turns out we could
overflow when scaling up one of the values.

In this patch we fix the problem by using a larger data type when we
detect that the scaled up value will not fit into the original data
type.

Testing:
- Added some expr tests that reproduce the issue.

Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.h
M be/src/runtime/decimal-value.inline.h
M be/src/util/bit-util.h
4 files changed, 121 insertions(+), 74 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/8833/2
--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8833 )

Change subject: IMPALA-6300: Fix decimal modulo overflow
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@190
PS1, Line 190:   int y_lz = BitUtil::CountLeadingZeros(abs(y));
> Ouch
?


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@261
PS1, Line 261: #ifdef DEBUG
> Is there some way we can avoid having different control flow on release and
Done. The compiler should optimize it away if it's false, right?


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@361
PS1, Line 361: RESULT_T x = 0;
> Unnecessary change - can you revert?
Done


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@571
PS1, Line 571: result = x % y;
> I think it would probably be faster and simpler to just use 64-bit arithmet
Done. I ran a benchmark and it turns that it is indeed better to simply use 
64-bit arithmetic.


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@641
PS1, Line 641:   return true;
> Since this can never be true if callers respect the comment above, maybe th
Good idea. We can simply remove this. The DCHECK is inside SafeMultiply().



--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 15 Dec 2017 02:42:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5014: Part 1: Round when casting string to decimal

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8774 )

Change subject: IMPALA-5014: Part 1: Round when casting string to decimal
..


Patch Set 2:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/runtime/decimal-test.cc
File be/src/runtime/decimal-test.cc:

http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/runtime/decimal-test.cc@302
PS2, Line 302:   StringToAllDecimals("0.5", 5, 0,
> More interesting test cases:
Done


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h
File be/src/util/string-parser.h:

http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@191
PS2, Line 191: } else if (UNLIKELY(round && total_digits_count == 
type_precision)) {
> This doesn't need to be conditional on round, in fact I think leaving that
We are saving the first truncated digit here. We save the truncated digit in 
order to be able to round if necessary. If round is false, there is no point in 
saving the first truncated digit.

Why do you think it's better for it to not be conditional on round?

Also, which DCHECK are you talking about? the one on line 194?


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@238
PS2, Line 238: // There are less than maximum number of digits to the 
left of the dot.
> Don't you mean "There are more than the maximum number of digits to the rig
We already know that there are too many digits to the right of the dot at this 
point because of comment on line 232. (which means we will be doing rounding 
for sure if round=true)

We are now checking on line 237 what's going on to the left of the dot. In this 
case, the number looks like this: x. with let's say prec=3, scale=1. If the 
number looked like this xx., the condition on line 237 would be false. (In 
other words, we are checking if the part of the number to the left of the dot 
is "full" or not on line 237).

If it's full, then we need to look at the first truncated digit. Otherwise, we 
can round without having to look at the first truncated digit.


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@239
PS2, Line 239: value = DecimalUtil::ScaleDownAndRound(value, shift, 
round);
> After thinking on this some more, rounding away from zero is symmetric with
Agreed. No changes made.


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@243
PS2, Line 243: // There are a maximum number of digits to the left of 
the dot. We attempt
> to the right of the dot?
No, to the left. See comment on line 238.


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@248
PS2, Line 248: DCHECK(first_truncated_digit == 0 || round);
> I found this DCHECK confusing until I realized saving the truncated digit w
It's still not clear to me why it shouldn't be conditional on round. Look at my 
comment above.


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@250
PS2, Line 250: value += (first_truncated_digit >= 5);
> This rounds towards positive infinity which isn't consistent with our round
Correct. We negate at the very bottom on line 266. So we are rounding away from 
zero.


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@262
PS2, Line 262:   DCHECK_EQ(truncated_digit_count, 0);
> It would be nice to have a DCHECK that this can't overflow.  While that is
Done


http://gerrit.cloudera.org:8080/#/c/8774/2/be/src/util/string-parser.h@391
PS2, Line 391:   int digits_after_dot_count, int type_precision, int8_t 
exponent, T* value,
> type_precision is unused in this function
Nice catch, removed.



--
To view, visit http://gerrit.cloudera.org:8080/8774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Gerrit-Change-Number: 8774
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 15 Dec 2017 22:09:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8833 )

Change subject: IMPALA-6300: Fix decimal modulo overflow
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/exprs/expr-test.cc@1574
PS1, Line 1574:   {{ false, false, 8892800, 12, 2 }}},
> Not sure where this came from, but okay.  Did it leak in from another diff?
Oh, it's a random bug I ran into in the code that I've just written when 
developing this patch. (All tests passed, but there was still a bug, and this 
test reproduces it, so I thought might as well add it).


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/exprs/expr-test.cc@2479
PS1, Line 2479:   { "cast(1 as decimal(6,1)) % cast(2 as decimal(37,35))",
> Did this one overflow before your change?
Yes it did (I just tried it to confirm)


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@145
PS1, Line 145: inline T SafeMultiply(T a, T b) {
> How about making this void `CheckMultiply(T a, T b)` and then just doing th
Took your most recent suggestion, and left the way it is in patch 2.


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@190
PS1, Line 190:   int y_lz = BitUtil::CountLeadingZeros(abs(y));
> This was counting in 64 bit all the time before, right?  Ouch because it wa
I guess it depended on the output of the abs() function. And I think the output 
of abs() is the same type as the input. So it was still probably doing the 
right thing.

It's better to make it explicit though, which is what I'm doing here.


http://gerrit.cloudera.org:8080/#/c/8833/1/be/src/runtime/decimal-value.inline.h@261
PS1, Line 261: #ifdef DEBUG
> Then you can omit the #ifdef here and just call CheckMultiply(left, mult) i
Left it unchanged after patch 2.



--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 15 Dec 2017 22:24:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8833


Change subject: IMPALA-6300: Fix decimal modulo overflow
..

IMPALA-6300: Fix decimal modulo overflow

In order to compute the modulo of two decimals, we need to bring the
underlying datatype to the same scale first. It turns out we could
overflow when scaling up one of the values.

In this patch we fix the problem by using a larger data type when we
detect that the scaled up value will not fit into the original data
type.

Testing:
- Added some expr tests that reproduce the issue.

Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/util/bit-util.h
3 files changed, 135 insertions(+), 49 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/8833/1
--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Standardize column alias behavior
..


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8801/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8801/4//COMMIT_MSG@13
PS4, Line 13: === Allowed ===
Nice, thanks for the examples!


http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@813
PS4, Line 813:  is false
I think it's better to say what happens when it's true. (Right now there is a 
double negative in this sentence).


http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@838
PS4, Line 838:   return trySubstitute(smap, analyzer, preserveRootType, 
true);
Maybe mention why this is always true in the method comment?


http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@850
PS4, Line 850:   result.add(e.trySubstitute(smap, analyzer, 
preserveRootTypes, true));
Maybe add a brief comment why it's always true?


http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@869
PS4, Line 869:* If substituteChildren is false, child expressions of 'this' 
will not be substituted.
Modify comment to remove the double negative.


http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/8801/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@303
PS4, Line 303: substituteExpr = expr.trySubstitute(aliasSmap_, 
analyzer, false, substituteChildren);
long line


http://gerrit.cloudera.org:8080/#/c/8801/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/8801/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@963
PS4, Line 963: bool_col
maybe make this "not bool_col" so that it's an expression?



--
To view, visit http://gerrit.cloudera.org:8080/8801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 12 Dec 2017 21:53:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3436: Return a decimal when rounding a double

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has abandoned this change. ( 
http://gerrit.cloudera.org:8080/8398 )

Change subject: IMPALA-3436: Return a decimal when rounding a double
..


Abandoned

We decided that rounding a double should return a double (instead of a decimal).
--
To view, visit http://gerrit.cloudera.org:8080/8398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I240ec70da7996bbfefcf6438eba4dff8d33d7bd6
Gerrit-Change-Number: 8398
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Standardize column alias behavior
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8801/5/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/8801/5/fe/src/main/java/org/apache/impala/analysis/Expr.java@817
PS5, Line 817:  boolean preserveRootType, boolean substituteChildren
> I find it a bit hard to read code calling methods with multiple boolean arg
Agreed. I like trySubstituteRecursive() and trySubstituteRootOnly()



--
To view, visit http://gerrit.cloudera.org:8080/8801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 13 Dec 2017 22:26:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8309 )

Change subject: IMPALA-5019: Decimal V2 addition
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402
PS4, Line 2402: 
> Still not following. The old V2 expected value was 999.6 not 999.5.
Regarding the first point:
the type of "power(10, 3) - 0.45" in decimal v2 before this patch is 
decimal(38, 17).
the type of "power(10, 3) - 0.45" in decimal v2 after this patch is decimal(38, 
16).

This causes the result of the conversion to double to be slightly different.

Before this patch (in decimal v2) after converting to double it looks like 
this: 999.5500156861889
After this patch, it looks like this: 999.5499727558438

This is why the result was 999.5 and not 999.6 after the patch.

This is all due to IMPALA-6183, which was fixed (and is reflected in the latest 
patch).



--
To view, visit http://gerrit.cloudera.org:8080/8309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336
Gerrit-Change-Number: 8309
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 20 Nov 2017 21:34:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 9:

(20 comments)

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/expr-test.cc@4653
PS9, Line 4653:   TestValue("width_bucket(22, 5, 20.1, 4)", TYPE_BIGINT, 5);
Add some tests with nulls.

For example:
(NULL, 5, 20, 3)
(12, NULL, 20, 3)
(12, 5, NULL, 3)
(12, 5, 20, NULL)


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@455
PS9, Line 455: min_range = -1 (or any negative value)
You can just write:
min_range < 0


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460
PS9, Line 460: Incase
"In case"


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460
PS9, Line 460:  t
extra space here


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460
PS9, Line 460: functions
function*


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@461
PS9, Line 461: decimal8Value
I am not sure it makes sense to even mention decimal8Value here. Why would you 
even think about storing it in a decimal 8?


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@461
PS9, Line 461: stroring
spelling


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@480
PS9, Line 480:   if (min_range == max_range) {
What if min_range > max_range?

Also, add a test case.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@481
PS9, Line 481: Lower bound cannot be equal to upper bound
Would it make sense to include the name of the function in the error message? 
Do we normally do that?


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@489
PS9, Line 489: num_buckets.val;
how about: num_buckets.val + 1

Also, add a test case where num_buckets is a maximally large integer, so adding 
one causes an overflow.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@494
PS9, Line 494:   // FE casts all the arguments to the same type.
Maybe expand this comment a little? E.g

"expr", "min_range" and "max_range" are guaranteed to be the same scale and 
precision by the FE


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@503
PS9, Line 503:   if(overflow) {
Have we considered what happens when subtraction is successful (and overflow is 
false), but the result is larger than the largest allowed decimal (10^38 - 1). 
Add a test case like this.

For example:
min range = -100
max_range = 10^38 - 1

The result of that subtraction should not overflow, because it fits into 
int128_t


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@514
PS9, Line 514:   if(overflow) {
Can this overflow ever happen without overflowing on line 503? Add a test case 
for this overflow and error message. (You might need to add some End to end 
tests to test for the error message)


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@533
PS9, Line 533:   if(needs_int256) {
Nit: Put a space between "if" and the opening brace. (here and elsewhere)


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@539
PS9, Line 539: avoid
avoid written twice


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@549
PS9, Line 549: // bump up the denominator scale so that the scale of the 
numerator and denominator
Can you explain this a little better? It's not really clear why we have to 
scale it up.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@551
PS9, Line 551: int128_t y = 
DecimalUtil::MultiplyByScale(range_size.value(),
Is it possible for this to overflow?


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@557
PS9, Line 557: ctx->SetError("Overflow while dividing by the range_size");
This should be moved up to around line 543.

Add a test case for this error message.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@571
PS9, Line 571:   if (UNLIKELY(num_buckets.val <= 0)) {
Why add this check here, instead of around line 480 where other similar check 
are located?


http://gerrit.cloudera.org:8080/#/c/6023/9/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/6023/9/fe/src/main/java/org/apache/impala/analysis/Expr.java@459
PS9, Line 459:
Why this empty line?



--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit 

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@503
PS9, Line 503:   if(overflow) {
> Have we considered what happens when subtraction is successful (and overflo
Actually I'm not correct here. It does overflow in the case I provided.

Wouldn't hurt to add the test case anyways.



--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 9
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 21 Nov 2017 01:43:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5936: operator '%' overflows on large decimals

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8574 )

Change subject: IMPALA-5936: operator '%' overflows on large decimals
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8574/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8574/2/be/src/exprs/decimal-operators-ir.cc@675
PS2, Line 675:   case 4: { \
It's probably a good idea to DCHECK here that x_size <= 4 and y_size <= 4


http://gerrit.cloudera.org:8080/#/c/8574/2/be/src/exprs/decimal-operators-ir.cc@684
PS2, Line 684: Decimal8Value x_val = GetDecimal8Value(x, x_size, 
); \
Maybe DCHECK that x_size <= 8 and y_size <= 8, to make sure where are not 
running into the same issue here.



--
To view, visit http://gerrit.cloudera.org:8080/8574
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b06c8acd5aa490943e84013faf2eaac7c26ceb4
Gerrit-Change-Number: 8574
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 21 Nov 2017 19:56:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4964: Fix Decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/8329 )

Change subject: IMPALA-4964: Fix Decimal modulo overflow
..

IMPALA-4964: Fix Decimal modulo overflow

The modulo operation between two decimals should never overflow. Before
this patch, there would be an overflow if the scale difference between
the two decimals was large. We would try to scale up the one with the
smaller scale, so that the scales matched, which could result in an
overflow.

We fix the problem by first checking if the scaled up value would fit
into 128 bits by estimating the number of leading zeros in it. If we
detect that 128 bits is not enough, we convert both numbers to int256,
do the operation, then convert back to 128 bits.

Testing:
- Added some expr tests that excercise the new code path.

Change-Id: I5420201d4440d421e33e443df005cdcc16b8a6cd
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-test.cc
M be/src/runtime/decimal-value.inline.h
3 files changed, 129 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8329/4
--
To view, visit http://gerrit.cloudera.org:8080/8329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5420201d4440d421e33e443df005cdcc16b8a6cd
Gerrit-Change-Number: 8329
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-3436: Return a decimal when rounding a double

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8398 )

Change subject: IMPALA-3436: Return a decimal when rounding a double
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:

http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@285
PS3, Line 285:   [['round_v1','dround_v1'],
> I agree could be legit uses cases for non-const precisions.
Just to clarify. I think what Alex is saying is that the proper fix is so that 
the round function looks like this: round(double, int) -> decimal AND the 
second argument can be non-const. The limitation in this patch is that the 
second arg must be const. Alex is also saying that there should not exist a 
round() function that returns a double in decimal v2 (and I agree with this).



--
To view, visit http://gerrit.cloudera.org:8080/8398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I240ec70da7996bbfefcf6438eba4dff8d33d7bd6
Gerrit-Change-Number: 8398
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 16 Nov 2017 06:15:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/8309 )

Change subject: IMPALA-5019: Decimal V2 addition
..

IMPALA-5019: Decimal V2 addition

In this patch, we implement the new decimal return type rules for
addition expressions. These rules become active when the query option
DECIMAL_V2 is enabled. The algorithm for determining the type of the
result is described in the JIRA.

DECIMAL V1:
++
| typeof(cast(1 as decimal(38,0)) + cast(0.1 as decimal(38,38))) |
++
| DECIMAL(38,38) |
++

DECIMAL V2:
++
| typeof(cast(1 as decimal(38,0)) + cast(0.1 as decimal(38,38))) |
++
| DECIMAL(38,6)  |
++

This patch required backend changes. We implement an algorithm where
we handle the whole and fractional parts separately, and then combine
them to get the final result. This is more complex and slower. We try
to avoid this by first checking if the result would fit into int128.

Testing:
- Added expr tests.
- Tested locally on my machine with a script that generates random
  decimal numbers and checks that Impala adds them correctly.

Performance:

For the common case, performance remains the same.
  select cast(2.2 as decimal(18, 1) + cast(2.2 as decimal(18, 1)

  BEFORE: 4.74s
  AFTER:  4.73s

In this case, we check if it is necessary to do the complex addition,
and it turns out to be not necessary. We see a slowdown because the
result needs to be scaled down by dividing.
  select cast(2.2 as decimal(38, 19) + cast(2.2 as decimal(38, 19)

  BEFORE: 1.63s
  AFTER:  13.57s

In following case, we take the most complex path and see the most
signification perfmance hit.
  select cast(7.5 as decimal(38,37)) + cast(2.2 as decimal(38,37))

  BEFORE: 1.63s
  AFTER: 20.57

Change-Id: I401049c56d910eb1546a178c909c923b01239336
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.h
M be/src/runtime/decimal-value.inline.h
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
5 files changed, 386 insertions(+), 76 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/8309/5
--
To view, visit http://gerrit.cloudera.org:8080/8309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336
Gerrit-Change-Number: 8309
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8309 )

Change subject: IMPALA-5019: Decimal V2 addition
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402
PS4, Line 2402: 
> why does this rounding no longer happen in decimal v2?
This went away after I fixed IMPALA-6183.

What was happening is that power(10, 3) - 0.45 returned a decimal in Decimal V2 
and was being converted to a double, which was imprecise. After converting back 
to decimal, rounding didn't happen because the value happened to be a little 
smaller than necessary for rounding.

I rebased the patch and fixed the conflicts.



--
To view, visit http://gerrit.cloudera.org:8080/8309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336
Gerrit-Change-Number: 8309
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 18 Nov 2017 00:24:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8309 )

Change subject: IMPALA-5019: Decimal V2 addition
..


Patch Set 5: Code-Review+1

Carrying the +1 from Tim


--
To view, visit http://gerrit.cloudera.org:8080/8309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336
Gerrit-Change-Number: 8309
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 18 Nov 2017 00:27:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3436: Return a decimal when rounding a double

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8398 )

Change subject: IMPALA-3436: Return a decimal when rounding a double
..


Patch Set 4: Code-Review-1

The current plan is to go in a completely different direction. We want to 
implement the following rule for round() functions:
The output type of a round() function must match the input type. This patch 
does the opposite.

I will abandon this patch next week, after thinking about it some more.


--
To view, visit http://gerrit.cloudera.org:8080/8398
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I240ec70da7996bbfefcf6438eba4dff8d33d7bd6
Gerrit-Change-Number: 8398
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 18 Nov 2017 00:36:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8309 )

Change subject: IMPALA-5019: Decimal V2 addition
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402
PS4, Line 2402: 
> But 1000 - 0.45 = 999.55. When casting that to DECIMAL(4,1), why doesn't th
the result of power(10, 3) - 0.45 was a double. Converted to decimal it looked 
like this: 999.5500 (decimal(38,16)). Converted back to double it 
was something like 999.499 (due to IMPALA-6183), then converting to decimal 
made it 999.5. This is no longer the case.



--
To view, visit http://gerrit.cloudera.org:8080/8309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336
Gerrit-Change-Number: 8309
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 18 Nov 2017 00:41:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8034 )

Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8034/17//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8034/17//COMMIT_MSG@7
PS17, Line 7: :U
nit: put a space after the semi colon



--
To view, visit http://gerrit.cloudera.org:8080/8034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299
Gerrit-Change-Number: 8034
Gerrit-PatchSet: 17
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 08 Dec 2017 00:57:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6292: Fix incorrect DCHECK in decimal subtraction

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8796


Change subject: IMPALA-6292: Fix incorrect DCHECK in decimal subtraction
..

IMPALA-6292: Fix incorrect DCHECK in decimal subtraction

When subtracting two decimals, and one of them is large, we incorrectly
hit a DCHECK if the intermediate result was equal to 10^39-1, which is
MAX_UNSCALED_DECIMAL16. We fix the problem by changing the condition in
the DCHECK from "less than" to "less than or equal to".

Testing:
- Added expr tests that reproduce the issue.

Change-Id: I42d42ad85efe32b7a0db0d2353385939fee09934
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
2 files changed, 19 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/8796/1
--
To view, visit http://gerrit.cloudera.org:8080/8796
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I42d42ad85efe32b7a0db0d2353385939fee09934
Gerrit-Change-Number: 8796
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-5014: Part 1: Round when casting string to decimal

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8774


Change subject: IMPALA-5014: Part 1: Round when casting string to decimal
..

IMPALA-5014: Part 1: Round when casting string to decimal

In this patch we implement rounding when casting string to decimal if
DECIMAL_V2 is enabled. The backend method that parses strings and
converts them to decimals is refactored to make it easier to understand.

Testing:
- Added some BE tests.

Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
---
M be/src/benchmarks/atod-benchmark.cc
M be/src/benchmarks/overflow-benchmark.cc
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/text-converter.inline.h
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/util/decimal-util.h
M be/src/util/string-parser.cc
M be/src/util/string-parser.h
11 files changed, 233 insertions(+), 101 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/8774/1
--
To view, visit http://gerrit.cloudera.org:8080/8774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Gerrit-Change-Number: 8774
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-5191: Behavior of column aliases should be more standard conforming

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Behavior of column aliases should be more standard 
conforming
..


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8801/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8801/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-5191: Behavior of column aliases should be more standard 
conforming
You should try to use the imperative mood in the subject line. For example, 
"Standardize column alias behavior"


http://gerrit.cloudera.org:8080/#/c/8801/3//COMMIT_MSG@11
PS3, Line 11: to be more standard conformant.
Add an example in the commit message of what is no longer allowed.


http://gerrit.cloudera.org:8080/#/c/8801/3/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/8801/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@813
PS3, Line 813: it won't substitute
"If onlyTopLevel is true, child expressions of 'this' will not be substituted."


http://gerrit.cloudera.org:8080/#/c/8801/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@833
PS3, Line 833:* the type of 'this'.
Add comment describing the new parameter.


http://gerrit.cloudera.org:8080/#/c/8801/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@836
PS3, Line 836: onlyTopLevel
I thought about this a little, and I think a more intuitive name for this 
parameter is
"substituteChildren", or something similar. What do you think?


http://gerrit.cloudera.org:8080/#/c/8801/3/fe/src/main/java/org/apache/impala/analysis/Expr.java@880
PS3, Line 880: onlyTopLevel
Isn't this always false?


http://gerrit.cloudera.org:8080/#/c/8801/3/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/8801/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@961
PS3, Line 961:  group by x"
It would be good to have a positive test with a HAVING clause. Maybe add 
another case where we have "group by x having x" and x is a boolean? (It won't 
work if it's not a boolean, right?)



--
To view, visit http://gerrit.cloudera.org:8080/8801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 Dec 2017 01:11:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5014: Part 1: Round when casting string to decimal

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8774 )

Change subject: IMPALA-5014: Part 1: Round when casting string to decimal
..


Patch Set 1:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/8774/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8774/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-5014: Part 1: Round when casting string to decimal
> What's left for later parts?
There's still decimal -> timestamp. It's not going to be a large patch, but I 
think it's better to separate them to make it easier to review.


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/exec/hdfs-scanner-ir.cc
File be/src/exec/hdfs-scanner-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/exec/hdfs-scanner-ir.cc@143
PS1, Line 143: Decimal4Value IrStringToDecimal4(const char* s, int len, int 
type_precision,
> Should we also be switching to rounding instead of truncation for text scan
Yes, on line 146, the result can't be PARSE_UNDERFLOW. So there is no point in 
trying to round.


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h
File be/src/util/string-parser.h:

http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@122
PS1, Line 122:   static inline void ApplyExponent(int total_digits_count,
> Should this be private? I don't think it makes sense for anything external
Done


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@123
PS1, Line 123:   int digits_after_dot_count, int type_precision, int8_t 
exponent, T& value,
> Our convention is to pass by pointer if it's modified.
Done


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@125
PS1, Line 125:
> nit unnecessary vertical whitespace
Done


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@127
PS1, Line 127:   // Ex: 0.1e3 (which at this point would have precision == 
1 and scale == 1), the
> Thanks for all the examples, this helped a lot when reviewing it.
Many of them were there already. I just moved this code into a separate 
function to make it easier to reason.


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@141
PS1, Line 141: if (*precision < *scale) *precision = *scale;
> Can this be moved into the else branch? If *scale is zero, this shouldn't b
Good catch. Moved.

If scale is zero, then precision has to be negative for this to be triggered. 
This should not happen.


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@204
PS1, Line 204: &
> Remove the reference here? We want value semantics anyway.
Done


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@219
PS1, Line 219: DCHECK
> DCHECK_GE?
Unfortunately DCHECK_GE does not work with int128_t. Added comment.


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@268
PS1, Line 268: maximumum
> maximum
Done


http://gerrit.cloudera.org:8080/#/c/8774/1/be/src/util/string-parser.h@620
PS1, Line 620: &
> Related to my other comment, passing a char value by reference is weird sin
Done. Passing by value now.



--
To view, visit http://gerrit.cloudera.org:8080/8774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Gerrit-Change-Number: 8774
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 Dec 2017 03:57:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5014: Part 1: Round when casting string to decimal

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8774 )

Change subject: IMPALA-5014: Part 1: Round when casting string to decimal
..

IMPALA-5014: Part 1: Round when casting string to decimal

In this patch we implement rounding when casting string to decimal if
DECIMAL_V2 is enabled. The backend method that parses strings and
converts them to decimals is refactored to make it easier to understand.

Testing:
- Added some BE tests.

Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
---
M be/src/benchmarks/atod-benchmark.cc
M be/src/benchmarks/overflow-benchmark.cc
M be/src/exec/hdfs-scanner-ir.cc
M be/src/exec/text-converter.inline.h
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/util/decimal-util.h
M be/src/util/string-parser.cc
M be/src/util/string-parser.h
11 files changed, 235 insertions(+), 103 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/8774/2
--
To view, visit http://gerrit.cloudera.org:8080/8774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Gerrit-Change-Number: 8774
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..


Patch Set 5: Code-Review+2

Added Cherry-picks: not for 2.x
Forwarding the +2.
Thanks, Phil!


--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 04 May 2018 21:42:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..

IMPALA-6949: Add the option to start the minicluster with EC enabled

In this patch we add the "ERASURE_CODING" enviornment variable. If we
enable it, a cluster with 5 data nodes will be created during data
loading and HDFS will be started with erasure coding enabled.

Testing:
I ran the core build, and verified that erasure coding gets enabled in
HDFS. Many of our EE tests failed however.

Cherry-picks: not for 2.x

Change-Id: I397aed491354be21b0a8441ca671232dca25146c
---
M bin/impala-config.sh
M bin/run-all-tests.sh
M testdata/bin/create-load-data.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/admin
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
6 files changed, 41 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10275/5
--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-6522: [DOCS] Document Decimal V2

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10066 )

Change subject: IMPALA-6522: [DOCS] Document Decimal V2
..


Patch Set 9:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10066/9/docs/topics/impala_decimal.xml
File docs/topics/impala_decimal.xml:

http://gerrit.cloudera.org:8080/#/c/10066/9/docs/topics/impala_decimal.xml@526
PS9, Line 526: 38,0
We should be consistent about putting a space after the comma. Either put it 
everywhere in the DOC or don't put it everywhere.


http://gerrit.cloudera.org:8080/#/c/10066/9/docs/topics/impala_decimal.xml@654
PS9, Line 654: is the INT
 : type with the precision 10.
... because all digits do not fit into DECIMAL(3,0)


http://gerrit.cloudera.org:8080/#/c/10066/9/docs/topics/impala_decimal.xml@681
PS9, Line 681:
There should be no space before the open brace. Here and elsewhere.



--
To view, visit http://gerrit.cloudera.org:8080/10066
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic436ff80c9ad05cfada97280cd47552879214a3d
Gerrit-Change-Number: 10066
Gerrit-PatchSet: 9
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 27 Apr 2018 19:21:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Speed up Python dependencies.

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10234 )

Change subject: Speed up Python dependencies.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10234/1/infra/python/deps/pip_download.py
File infra/python/deps/pip_download.py:

http://gerrit.cloudera.org:8080/#/c/10234/1/infra/python/deps/pip_download.py@165
PS1, Line 165: results.append(pool.apply_async(download_package, 
args=[pkg_name.strip(), pkg_version.strip()]))
long line


http://gerrit.cloudera.org:8080/#/c/10234/1/infra/python/deps/pip_download.py@173
PS1, Line 173: x.get()
What happens if the download fails for some reason? Will this be detected?



--
To view, visit http://gerrit.cloudera.org:8080/10234
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cbf622adb7d037f1a53c519402dcd8ae3c0fe30
Gerrit-Change-Number: 10234
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 27 Apr 2018 20:03:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6522: [DOCS] Document Decimal V2

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10066 )

Change subject: IMPALA-6522: [DOCS] Document Decimal V2
..


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml
File docs/topics/impala_decimal.xml:

http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@539
PS10, Line 539: OUBLE
DOUBLE and FLOAT


http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@540
PS10, Line 540: error returns.
Maybe add the following.

The reasoning is that the range of the floating point types is much wider than 
the range of the DECIMAL, so not every FLOAT or DOUBLE can be converted to a 
DECIMAL. However, every DECIMAL can be converted to a double or float with a 
loss of precision, which is why we allow implicit casting in this case. This is 
somewhat dangerous, but we made this design choice to match the behavior of 
other SQL engines.


http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@545
PS10, Line 545:
Add this:
We do not allow this because calculations involving decimals are meant to be 
precise, so we are strict and require explicit casts. However, if an 
approximate type like FLOAT is involved, then our behavior is less strict.



--
To view, visit http://gerrit.cloudera.org:8080/10066
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic436ff80c9ad05cfada97280cd47552879214a3d
Gerrit-Change-Number: 10066
Gerrit-PatchSet: 10
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 27 Apr 2018 21:59:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Speed up Python dependencies.

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10234 )

Change subject: Speed up Python dependencies.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10234/2/infra/python/deps/pip_download.py
File infra/python/deps/pip_download.py:

http://gerrit.cloudera.org:8080/#/c/10234/2/infra/python/deps/pip_download.py@172
PS2, Line 172: if results:
> If the import above fails, then results = [] will never execute and this st
Nice catch. We can get rid of the "if results:" line



--
To view, visit http://gerrit.cloudera.org:8080/10234
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cbf622adb7d037f1a53c519402dcd8ae3c0fe30
Gerrit-Change-Number: 10234
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 27 Apr 2018 22:03:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Speed up Python dependencies.

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10234 )

Change subject: Speed up Python dependencies.
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/10234
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cbf622adb7d037f1a53c519402dcd8ae3c0fe30
Gerrit-Change-Number: 10234
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Fri, 27 Apr 2018 21:34:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6522: [DOCS] Document Decimal V2

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10066 )

Change subject: IMPALA-6522: [DOCS] Document Decimal V2
..


Patch Set 10:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml
File docs/topics/impala_decimal.xml:

http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@50
PS10, Line 50:   This data type is suitable for financial and other 
arithmetic calculations where the
We should have a better introduction. We would not be comparing it to FLOAT. We 
should not mention that it's suitable for financial purposes.

How about this:
Precision is the maximum number of decimal digits and scale is the number of 
digits to the right of the decimal point. The data type is useful for storing 
and doing operations on precise decimal values.


http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@56
PS10, Line 56:   The key differences between this version of 
DECIMAL and the previous
We should move this table to the bottom. Instead of calling it DECIMAL_V1 and 
DECIMAL_v2 we should say "Decimal in Impala 2.x" and "Decimal in Impala 3.x". 
Say that the old behavior can be enabled by disabling the query option "set 
decimal_v2=false"


http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@293
PS10, Line 293:   If the precision of the result would be greater than 38, 
Impala truncates the result from
> truncates and rounds, right Taras?
yes, "truncates and rounds"

Let's make a separate section called "Decimal Assignment" as Alex mentioned and 
put UNION in that section.


http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@668
PS10, Line 668:   expressions to DECIMAL as long as the 
overall number of digits and digits
> Taras?
Alex is right. This contradicts what it says on line 678.

should be "... as number fits within the specified target DECIMAL type without 
overflow"


http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@879
PS10, Line 879: Any values that do not fit within the new 
precision and scale are returned as
> Taras, is this really true? I would have expected that we round or error.
No, we do not round. We will error if the query option ABORT_ON_ERROR is set to 
true. If that option is set to FALSE, we will get a NULL and warning that 
conversion failed.



--
To view, visit http://gerrit.cloudera.org:8080/10066
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic436ff80c9ad05cfada97280cd47552879214a3d
Gerrit-Change-Number: 10066
Gerrit-PatchSet: 10
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Sat, 28 Apr 2018 01:09:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 5: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/10413
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Mon, 21 May 2018 21:33:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java@490
PS15, Line 490:   result = ((ScalarType)result).getMinResolutionDecimal();
Why do we need this?



--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 22 May 2018 00:17:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7079: Disable the multiple blocks test in erasure coding build

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10521


Change subject: IMPALA-7079: Disable the multiple blocks test in erasure coding 
build
..

IMPALA-7079: Disable the multiple blocks test in erasure coding build

The test is currently failing in erasure coding build, so disable it to
make the build pass.

Change-Id: I00af0914d907b8dcff69f687f71239e76b6ff335
---
M tests/query_test/test_scanners.py
1 file changed, 1 insertion(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/10521/1
--
To view, visit http://gerrit.cloudera.org:8080/10521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I00af0914d907b8dcff69f687f71239e76b6ff335
Gerrit-Change-Number: 10521
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10413/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/10413/4/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@129
PS4, Line 129: static FileDescriptor createEc(FileStatus fileStatus, 
BlockLocation[] blockLocations,
The logic here seems almost identical to the create() function. Maybe modify 
the create() function to accept the isEC argument instead of creating a new 
function?



--
To view, visit http://gerrit.cloudera.org:8080/10413
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 18 May 2018 23:05:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7039: Ignore the port in HBase planner tests

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10459


Change subject: IMPALA-7039: Ignore the port in HBase planner tests
..

IMPALA-7039: Ignore the port in HBase planner tests

Before this patch, we used to check the HBase port in the HBase planner
tests. This caused a failure when HBase was running on a different port
than expected. We fix the problem in this patch by not checking the
HBase port.

Testing: ran the FE tests and they passed.

Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
---
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
3 files changed, 20 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10459/1
--
To view, visit http://gerrit.cloudera.org:8080/10459
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
Gerrit-Change-Number: 10459
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10413/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10413/3//COMMIT_MSG@12
PS3, Line 12:
Mention how this patch was tested. Did you run the exhaustive build?


http://gerrit.cloudera.org:8080/#/c/10413/3/tests/common/skip.py
File tests/common/skip.py:

http://gerrit.cloudera.org:8080/#/c/10413/3/tests/common/skip.py@29
PS3, Line 29: IS_ISILON,
: IS_LOCAL,
: IS_HDFS,
: IS_S3,
: IS_ADLS,
: SECONDARY_FILESYSTEM,
: IS_EC
This should be sorted alphabetically


http://gerrit.cloudera.org:8080/#/c/10413/3/tests/common/skip.py@150
PS3, Line 150: doesn't work.
"do not work"


http://gerrit.cloudera.org:8080/#/c/10413/3/tests/util/filesystem_utils.py
File tests/util/filesystem_utils.py:

http://gerrit.cloudera.org:8080/#/c/10413/3/tests/util/filesystem_utils.py@33
PS3, Line 33: os.getenv("ERASURE_CODING")
We want to check if ERASURE_CODING == true here.



--
To view, visit http://gerrit.cloudera.org:8080/10413
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 18 May 2018 21:57:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7102: Disable support of erasure coding by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10646


Change subject: IMPALA-7102: Disable support of erasure coding by default
..

IMPALA-7102: Disable support of erasure coding by default

In this patch we add a query option ALLOW_ERASURE_CODED_FILES, that
allows us to enable or disable the support of erasure coded files. At
this time erasure coding is an experimental feature and is not supported
by Impala, so we want to prevent users from using it without explictly
enabling it.

Cherry-picks: not for 2.x

Change-Id: Icd3b1754541262467a6e67068b0b447882a40fb3
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/run-all-tests.sh
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/bin/create-load-data.sh
A testdata/workloads/functional-query/queries/QueryTest/hdfs-erasure-coding.test
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
M tests/query_test/test_observability.py
M tests/query_test/test_scanners.py
12 files changed, 64 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/10646/1
--
To view, visit http://gerrit.cloudera.org:8080/10646
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd3b1754541262467a6e67068b0b447882a40fb3
Gerrit-Change-Number: 10646
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-7149: Skip q7 in test mem usage scaling in erasure coding build

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10647


Change subject: IMPALA-7149: Skip q7 in test_mem_usage_scaling in erasure 
coding build
..

IMPALA-7149: Skip q7 in test_mem_usage_scaling in erasure coding build

The test is flaky in the erasure coding build. Let's disable it for now.

Change-Id: Ic9a34a91eef40e1da9c7134cfb7054006d9115de
---
M tests/query_test/test_mem_usage_scaling.py
1 file changed, 2 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10647/1
--
To view, visit http://gerrit.cloudera.org:8080/10647
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic9a34a91eef40e1da9c7134cfb7054006d9115de
Gerrit-Change-Number: 10647
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 20:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/20/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/20/be/src/exprs/math-functions-ir.cc@565
PS20, Line 565:   ctx->SetError("Encountered division by 0");
> clang fails with division by 0. Since we check for
This is not the right way to deal with this. I looked at it again, and I really 
don't think it's possible for y to be zero. This needless check will make the 
code slower.

Include this comment on the line that is causing the problem to suppress that 
clang error:

// NOLINT: clang-tidy thinks y may equal zero here.

You just need to put this on line 584 I think



--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 20
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 29 Jun 2018 21:54:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 21: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 21
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 30 Jun 2018 01:25:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10780 )

Change subject: IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10780/1/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/10780/1/bin/start-impala-cluster.py@460
PS1, Line 460: print 'Error starting cluster: {0}'.format(e)
> If you switch to logging, this becomes just "logging.exception("Error start
Done



--
To view, visit http://gerrit.cloudera.org:8080/10780
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
Gerrit-Change-Number: 10780
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 22 Jun 2018 23:02:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6642 (Part 2): clean up start-impala-cluster.py

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10780 )

Change subject: IMPALA-6642 (Part 2): clean up start-impala-cluster.py
..

IMPALA-6642 (Part 2): clean up start-impala-cluster.py

We clean up start-impala-cluster.py in general in this patch by using
logging instead of "print" and formatting strings using the format()
function. We make sure to include a timestamp in each log message in
order to make it easier to debug failures in custom cluster tests that
happen when starting the cluster.

Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
---
M bin/start-impala-cluster.py
M tests/common/impala_service.py
2 files changed, 147 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/10780/2
--
To view, visit http://gerrit.cloudera.org:8080/10780
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
Gerrit-Change-Number: 10780
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 16: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 16
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 22 Jun 2018 23:46:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7149: Disable some tests in the EC build

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10804


Change subject: IMPALA-7149: Disable some tests in the EC build
..

IMPALA-7149: Disable some tests in the EC build

We temporarily disable the resource limits tests in the EC build to
make it pass. We also disable the tests marked with
"tuned_for_minicluster" in the EC build.

Cherry-picks: not for 2.x.

Change-Id: I0975b1a28b318625f853b612bdfea3a8adcd776e
---
M tests/common/skip.py
M tests/query_test/test_resource_limits.py
2 files changed, 4 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/10804/1
--
To view, visit http://gerrit.cloudera.org:8080/10804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0975b1a28b318625f853b612bdfea3a8adcd776e
Gerrit-Change-Number: 10804
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

Also, I agree with Tim that it's a good idea to add this to the fuzz test. I 
think that it is ok to do that in a separate commit.


--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:57:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

I already let Anuj know offline that it's ok to remove the precondition.


--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:55:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10780


Change subject: IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py
..

IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

We add some timestamps to the output of start-impala-cluster.py in
order to make it easier to debug failures in custom cluster tests that
happen when starting the cluster.

Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
---
M bin/start-impala-cluster.py
M tests/common/impala_service.py
2 files changed, 22 insertions(+), 15 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/10780/1
--
To view, visit http://gerrit.cloudera.org:8080/10780
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
Gerrit-Change-Number: 10780
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-7102: Disable support of erasure coding by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10646 )

Change subject: IMPALA-7102: Disable support of erasure coding by default
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10646/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10646/1//COMMIT_MSG@9
PS1, Line 9: In this patch we add a query option ALLOW_ERASURE_CODED_FILES, that
> I had a high-level question: what's the rationale for making it a query opt
Updated the commit message.

The feature should work in principle and has been tested to a certain extent 
already and it works. It wouldn't hurt to make it possible for advanced users 
to be able to enabled the feature if they know what they are doing.


http://gerrit.cloudera.org:8080/#/c/10646/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/10646/1/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@786
PS1, Line 786:   "The query involves scanning an erasure coded file 
%s located in %s. " +
Updated the message



--
To view, visit http://gerrit.cloudera.org:8080/10646
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd3b1754541262467a6e67068b0b447882a40fb3
Gerrit-Change-Number: 10646
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Adrian Ng
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 27 Jun 2018 00:01:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7102(Part 1): Disable reading of erasure coding by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10646 )

Change subject: IMPALA-7102(Part 1): Disable reading of erasure coding by 
default
..

IMPALA-7102(Part 1): Disable reading of erasure coding by default

In this patch we add a query option ALLOW_ERASURE_CODED_FILES, that
allows us to enable or disable the support of erasure coded files. Even
though Impala should be able to handle HDFS erasure coded files already,
this feature hasn't been tested thoroughly yet. Also, Impala lacks
metrics, observability and DDL commands related to erasure coding. This
is a query option instead of a startup flag because we want to make it
possible for advanced users to enable the feature.

We may also need a follow on patch to also disable the write path with
this flag.

Cherry-picks: not for 2.x

Change-Id: Icd3b1754541262467a6e67068b0b447882a40fb3
---
M be/src/service/query-options.cc
M be/src/service/query-options.h
M bin/run-all-tests.sh
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/bin/create-load-data.sh
A testdata/workloads/functional-query/queries/QueryTest/hdfs-erasure-coding.test
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
M tests/query_test/test_observability.py
M tests/query_test/test_scanners.py
12 files changed, 61 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/10646/2
--
To view, visit http://gerrit.cloudera.org:8080/10646
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icd3b1754541262467a6e67068b0b447882a40fb3
Gerrit-Change-Number: 10646
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Adrian Ng
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7102 (Part 1): Disable reading of erasure coding by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10646 )

Change subject: IMPALA-7102 (Part 1): Disable reading of erasure coding by 
default
..


Patch Set 4: Code-Review+2

Fixed a typo in the test file. Forwarding the +2.


--
To view, visit http://gerrit.cloudera.org:8080/10646
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd3b1754541262467a6e67068b0b447882a40fb3
Gerrit-Change-Number: 10646
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Adrian Ng
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 29 Jun 2018 20:10:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10275


Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..

IMPALA-6949: Add the option to start the minicluster with EC enabled

In this patch we add the "hdfs-ec" target file system option. If we
enable it, a cluster with 5 data nodes will be created during data
loading and HDFS will be started with erasure coding enabled.

Change-Id: I397aed491354be21b0a8441ca671232dca25146c
---
M bin/impala-config.sh
M bin/run-all-tests.sh
M buildall.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/admin
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
6 files changed, 24 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10275/1
--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..

IMPALA-6949: Add the option to start the minicluster with EC enabled

In this patch we add the "hdfs-ec" target file system option. If we
enable it, a cluster with 5 data nodes will be created during data
loading and HDFS will be started with erasure coding enabled.

Testing:
I ran the core build and the majority of the EE tests passed.

Change-Id: I397aed491354be21b0a8441ca671232dca25146c
---
M bin/impala-config.sh
M bin/run-all-tests.sh
M buildall.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/admin
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
6 files changed, 24 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10275/2
--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..

IMPALA-6949: Add the option to start the minicluster with EC enabled

In this patch we add the "ERASURE_CODING" enviornment variable. If we
enable it, a cluster with 5 data nodes will be created during data
loading and HDFS will be started with erasure coding enabled.

Testing:
I ran the core build, and verified that erasure coding gets enabled in
HDFS. Many of our EE tests failed however.

Change-Id: I397aed491354be21b0a8441ca671232dca25146c
---
M bin/impala-config.sh
M bin/run-all-tests.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/admin
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
5 files changed, 28 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10275/3
--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..


Patch Set 2:

(5 comments)

Yes, nested data loading succeeds every time I tried it.

http://gerrit.cloudera.org:8080/#/c/10275/2/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/10275/2/bin/impala-config.sh@449
PS2, Line 449: elif [ "${TARGET_FILESYSTEM}" = "hdfs-ec" ]; then
> By making this its own TARGET_FILESYSTEM, you end up disabling a bunch of t
I don't think it makes sense to make erasure coding its own file system. I made 
a separate flag for it.


http://gerrit.cloudera.org:8080/#/c/10275/2/bin/impala-config.sh@454
PS2, Line 454:   echo "Valid values are: hdfs, isilon, s3, local"
> Please update?
No need to update any more.


http://gerrit.cloudera.org:8080/#/c/10275/2/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/10275/2/bin/run-all-tests.sh@71
PS2, Line 71:   FE_TEST=false
> Add a comment about why?
Done


http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/bin/setup-hdfs-env.sh
File testdata/bin/setup-hdfs-env.sh:

http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/bin/setup-hdfs-env.sh@80
PS2, Line 80:   hdfs ec -unsetPolicy -path "${HDFS_EC_PATH:=/test-warehouse/}"
> Do you know what this does underneath the covers? Do we need to block while
This does not convert the existing files in the directory. It only affects the 
files that will be placed in the directory in the future. This is why it is 
possible to have some files erasure coded and some not in the same directory.


http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
File testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl:

http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl@26
PS2, Line 26: cloudera.erasure_coding.enabled
> What's this for?
This is needed because running the following command

hdfs ec -enablePolicy -policy RS-3-2-1024k

Causes this error:

RemoteException: enableErasureCodingPolicy is not allowed because 
cloudera.erasure_coding.enabled=false



--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Wed, 02 May 2018 21:23:44 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP: IMPALA-6518,IMPALA-6340: Check that decimal types are compatible in FE

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9930 )

Change subject: WIP: IMPALA-6518,IMPALA-6340: Check that decimal types are 
compatible in FE
..


Patch Set 8:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java:

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@377
PS8, Line 377:   Analyzer analyzer, AnalyticWindow.Boundary boundary) 
throws AnalysisException {
> No need to pass analyzer.
Done


http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
File fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java:

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@217
PS8, Line 217: Preconditions.checkState(!t0.isDecimal() && 
!t1.isDecimal());
> remove
Done


http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/Expr.java@429
PS8, Line 429:* If strictDecimal is true, the function will throw an 
exception if it is not possible
> This isn't accurate because we will still allow decimal->double conversions
Done


http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java
File fe/src/main/java/org/apache/impala/analysis/StatementBase.java:

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@178
PS8, Line 178:* message.
> Needs comment for 'strictDecimal'
Done


http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/ScalarType.java
File fe/src/main/java/org/apache/impala/catalog/ScalarType.java:

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/ScalarType.java@433
PS8, Line 433:* is INVALID_TYPE.
> Especially these very visible and heavily used functions need a comment for
Done


http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/Type.java
File fe/src/main/java/org/apache/impala/catalog/Type.java:

http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/Type.java@293
PS8, Line 293:* If strictDecimal is true, only consider casts that result 
in no loss of information
> This is a nice description! I suggest replicating it in more places.
Done



--
To view, visit http://gerrit.cloudera.org:8080/9930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568
Gerrit-Change-Number: 9930
Gerrit-PatchSet: 8
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Thu, 26 Apr 2018 21:07:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6340,IMPALA-6518: Check that decimal types are compatible in FE

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#9). ( 
http://gerrit.cloudera.org:8080/9930 )

Change subject: IMPALA-6340,IMPALA-6518: Check that decimal types are 
compatible in FE
..

IMPALA-6340,IMPALA-6518: Check that decimal types are compatible in FE

In this patch we implement strict decimal type checking in the FE in
various situations when DECIMAL_V2 is enabled. What is affected:
- Union. If we union two decimals and it is not possible to come up
  with a decimal that will be able to contain all the digits, an error
  is thrown. For example, the union(decimal(20, 10), decimal(20, 20))
  returns decimal(30, 20). However, for union(decimal(38, 0),
  decimal(38, 38)) the ideal return type would be decimal(76,38), but
  this is too large, so an error is thrown.
- Insert. If we are inserting a decimal value into a column where we are
  not guaranteed that all digits will fit, an error is thrown. For
  example, inserting a decimal(38,0) value into a decimal(38,38) column.
- Functions such as coalesce(). If we are unable to determine the output
  type that guarantees that all digits will fit from all the arguments,
  an error is thrown. For example,
  coalesce(decimal(38,38), decimal(38,0)) will throw an error.
- Hash Join. When joining on two decimals, if a type cannot be
  determined that both columns can be cast to, we throw an error.
  For example, join on decimal(38,0) and decimal(38,38) will result
  in an error.

To avoid these errors, you need to use CAST() on some of the decimals.

In this patch we also change the output decimal calculation of decimal
round, truncate and related functions. If these functions are a no-op,
the resulting decimal type is the same as the input type.

Testing:
- Ran the BE and FE tests locally and they all passed.

Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568
---
M be/src/exprs/expr-test.cc
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/Function.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/complex-types-file-formats.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
34 files changed, 547 insertions(+), 292 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/9930/9
--
To view, visit http://gerrit.cloudera.org:8080/9930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568
Gerrit-Change-Number: 9930
Gerrit-PatchSet: 9
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-6340,IMPALA-6518: Check that decimal types are compatible in FE

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/9930 )

Change subject: IMPALA-6340,IMPALA-6518: Check that decimal types are 
compatible in FE
..

IMPALA-6340,IMPALA-6518: Check that decimal types are compatible in FE

In this patch we implement strict decimal type checking in the FE in
various situations when DECIMAL_V2 is enabled. What is affected:
- Union. If we union two decimals and it is not possible to come up
  with a decimal that will be able to contain all the digits, an error
  is thrown. For example, the union(decimal(20, 10), decimal(20, 20))
  returns decimal(30, 20). However, for union(decimal(38, 0),
  decimal(38, 38)) the ideal return type would be decimal(76,38), but
  this is too large, so an error is thrown.
- Insert. If we are inserting a decimal value into a column where we are
  not guaranteed that all digits will fit, an error is thrown. For
  example, inserting a decimal(38,0) value into a decimal(38,38) column.
- Functions such as coalesce(). If we are unable to determine the output
  type that guarantees that all digits will fit from all the arguments,
  an error is thrown. For example,
  coalesce(decimal(38,38), decimal(38,0)) will throw an error.
- Hash Join. When joining on two decimals, if a type cannot be
  determined that both columns can be cast to, we throw an error.
  For example, join on decimal(38,0) and decimal(38,38) will result
  in an error.

To avoid these errors, you need to use CAST() on some of the decimals.

In this patch we also change the output decimal calculation of decimal
round, truncate and related functions. If these functions are a no-op,
the resulting decimal type is the same as the input type.

Testing:
- Ran a core build which passed.

Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568
---
M be/src/exprs/expr-test.cc
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M fe/src/main/java/org/apache/impala/catalog/Function.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/TypesUtilTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/complex-types-file-formats.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
34 files changed, 550 insertions(+), 292 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/9930/10
--
To view, visit http://gerrit.cloudera.org:8080/9930
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568
Gerrit-Change-Number: 9930
Gerrit-PatchSet: 10
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..

IMPALA-6949: Add the option to start the minicluster with EC enabled

In this patch we add the "ERASURE_CODING" enviornment variable. If we
enable it, a cluster with 5 data nodes will be created during data
loading and HDFS will be started with erasure coding enabled.

Testing:
I ran the core build, and verified that erasure coding gets enabled in
HDFS. Many of our EE tests failed however.

Change-Id: I397aed491354be21b0a8441ca671232dca25146c
---
M bin/impala-config.sh
M bin/run-all-tests.sh
M testdata/bin/create-load-data.sh
M testdata/bin/setup-hdfs-env.sh
M testdata/cluster/admin
M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
6 files changed, 41 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/10275/4
--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10275 )

Change subject: IMPALA-6949: Add the option to start the minicluster with EC 
enabled
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10275/3/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/10275/3/bin/impala-config.sh@450
PS3, Line 450: elif [ "${TARGET_FILESYSTEM}" == "hdfs" ]; then
> I'm trying this out and found that == don't work with zsh.
Done


http://gerrit.cloudera.org:8080/#/c/10275/3/bin/impala-config.sh@451
PS3, Line 451:   if [[ "${ERASURE_CODING}" = true ]]; then
> I think we should error out or warn here if MINICLUSTER_PROFILE < 3. I.e.,
Done


http://gerrit.cloudera.org:8080/#/c/10275/3/testdata/bin/setup-hdfs-env.sh
File testdata/bin/setup-hdfs-env.sh:

http://gerrit.cloudera.org:8080/#/c/10275/3/testdata/bin/setup-hdfs-env.sh@80
PS3, Line 80: HDFS_EC_PATH
> Please add:
I agree. I don't think it makes sense to unset it here, because it's not going 
to do anything. Removed.


http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl
File testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl:

http://gerrit.cloudera.org:8080/#/c/10275/2/testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl@26
PS2, Line 26: cloudera.erasure_coding.enabled
> Add:
Done



--
To view, visit http://gerrit.cloudera.org:8080/10275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I397aed491354be21b0a8441ca671232dca25146c
Gerrit-Change-Number: 10275
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Fri, 04 May 2018 01:40:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7202: Add width bucket() to the decimal fuzz test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10859


Change subject: IMPALA-7202: Add width_bucket() to the decimal fuzz test
..

IMPALA-7202: Add width_bucket() to the decimal fuzz test

In this patch we include the newly added width_bucket() function in the
decimal fuzz test.

Change-Id: I1f8a63733ebddc07f46c525ca51ad4794dd0
---
M tests/query_test/test_decimal_fuzz.py
1 file changed, 57 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10859/1
--
To view, visit http://gerrit.cloudera.org:8080/10859
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1f8a63733ebddc07f46c525ca51ad4794dd0
Gerrit-Change-Number: 10859
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-6231: Implement decimal v2 fuzz test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8898 )

Change subject: IMPALA-6231: Implement decimal_v2 fuzz test
..

IMPALA-6231: Implement decimal_v2 fuzz test

Implement a test that generates random decimal numbers in the pytest
framework, performs a random mathemtaical operation in Impala and
verifies that the result is correct by doing the same operating using
the Python decimal module. We try to generate not only completely random
decimal numbers, but also numbers that have interesting properties, such
as the number being a power of two.

Change-Id: I4328125de5c583ec8ead1f78d9a08703b18b2d85
---
A tests/query_test/test_decimal_fuzz.py
1 file changed, 249 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/8898/2
--
To view, visit http://gerrit.cloudera.org:8080/8898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4328125de5c583ec8ead1f78d9a08703b18b2d85
Gerrit-Change-Number: 8898
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6231: Implement decimal v2 fuzz test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8898 )

Change subject: IMPALA-6231: Implement decimal_v2 fuzz test
..


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py
File tests/query_test/test_decimal_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@33
PS1, Line 33:   # Set the Python decimal context to a large precision 
initially, so that the
:   # mathematical operations are performed at a high precision.
:   decimal.getcontext().prec = 80
:   decimal.getcontext().rounding = decimal.ROUND_HALF_UP
> Use def setup_method and teardown_method for this?
I used local decimal context to solve the issue.


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@46
PS1, Line 46: create_exec_option_dimension_from_dict({'_': ['_']})
> Sorry, what does this do?
I'm just trying to remove all test dimensions that we normally use (such as 
file format). I'm just adding a useless dimension here, because the test 
doesn't run with zero dimensions. Is there a better way to do this?


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@65
PS1, Line 65: Returns a decimal with a random precision, scale and value.
> What about something a little more precise, like "Return a 3-tuple with str
Done


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@70
PS1, Line 70: 38
> I think we want 38
Yes, I think we want 38. It's ok that 38 shows up in both extreme precision and 
random precision tests. We want random to be completely random and all values 
should be possible.


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@134
PS1, Line 134: decimal_value
> It was a little confusing at first to know what the intention was here. May
Done


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@204
PS1, Line 204:   truncated_expected = expected.quantize(
> Nice.  I didn't know you could do this.  I tried a few of the more bizarre
Yeah, it's pretty cool. I learned about this while developing this test.


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@230
PS1, Line 230:   if op == '+':
 : expected_result = decimal.Decimal(value1) + 
decimal.Decimal(value2)
 :   elif op == '-':
 : expected_result = decimal.Decimal(value1) - 
decimal.Decimal(value2)
 :   elif op == '*':
 : expected_result = decimal.Decimal(value1) * 
decimal.Decimal(value2)
 :   elif op == '/':
 : expected_result = decimal.Decimal(value1) / 
decimal.Decimal(value2)
 :   elif op == '%':
 : expected_result = decimal.Decimal(value1) % 
decimal.Decimal(value2)
> Up to you, but you could consider the operator module so you don't have to
Are you suggesting to create a mapping m that maps, for example, "+" to 
operator.add()? Then calculate result like this m[op](value1, value2)?

I don't think doing this is much better than the current approach.


http://gerrit.cloudera.org:8080/#/c/8898/1/tests/query_test/test_decimal_fuzz.py@248
PS1, Line 248: def test_fuzz(self, vector):
> You could parallelize the test by making the iteration a test parameter.
Cool tip, but I decided against making each iteration a separate test. Too much 
stuff gets printed to screen.



--
To view, visit http://gerrit.cloudera.org:8080/8898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4328125de5c583ec8ead1f78d9a08703b18b2d85
Gerrit-Change-Number: 8898
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Sat, 06 Jan 2018 02:14:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/8833 )

Change subject: IMPALA-6300: Fix decimal modulo overflow
..

IMPALA-6300: Fix decimal modulo overflow

In order to compute the modulo of two decimals, we need to bring the
underlying datatype to the same scale first. It turns out we could
overflow when scaling up one of the values.

In this patch we fix the problem by using a larger data type when we
detect that the scaled up value will not fit into the original data
type.

Testing:
- Added some expr tests that reproduce the issue.

Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
---
M be/src/benchmarks/overflow-benchmark.cc
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.h
M be/src/runtime/decimal-value.inline.h
M be/src/util/bit-util.h
M be/src/util/decimal-util.h
6 files changed, 128 insertions(+), 135 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/8833/5
--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8833 )

Change subject: IMPALA-6300: Fix decimal modulo overflow
..


Patch Set 5: Code-Review+2

Made a minor change to overflow benchmark. Forwarding the +2. Thanks for the 
review, Zach!


--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 22 Dec 2017 08:45:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8833 )

Change subject: IMPALA-6300: Fix decimal modulo overflow
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8833/4/be/src/benchmarks/overflow-benchmark.cc
File be/src/benchmarks/overflow-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/8833/4/be/src/benchmarks/overflow-benchmark.cc@137
PS4, Line 137:   return false;
> This now always returns false.
Changed it so that the function returns nothing.


http://gerrit.cloudera.org:8080/#/c/8833/4/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

http://gerrit.cloudera.org:8080/#/c/8833/4/be/src/runtime/decimal-value.inline.h@186
PS4, Line 186:   return num_lz - floor_log2[scale_diff] - 1;
> num_lz - floor_log2[scale_diff] - (scale_diff > 0)
Interesting optimization for the case where scale_diff is zero. This isn't 
expected so decided to keep the code unmodified for simplicity.



--
To view, visit http://gerrit.cloudera.org:8080/8833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Gerrit-Change-Number: 8833
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 22 Dec 2017 08:44:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5014: Part 1: Round when casting string to decimal

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8774 )

Change subject: IMPALA-5014: Part 1: Round when casting string to decimal
..


Patch Set 4: Code-Review+2

Fixed the issue with the test. The test did not expected there to be rounding 
when converting string to decimal, that's why it was failing. Forwarding the +2.


--
To view, visit http://gerrit.cloudera.org:8080/8774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Gerrit-Change-Number: 8774
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 22 Dec 2017 07:57:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5014: Part 1: Round when casting string to decimal

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8774 )

Change subject: IMPALA-5014: Part 1: Round when casting string to decimal
..


Patch Set 3:

(2 comments)

Thanks for the review, Zach!

http://gerrit.cloudera.org:8080/#/c/8774/3/be/src/exec/hdfs-scanner-ir.cc
File be/src/exec/hdfs-scanner-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8774/3/be/src/exec/hdfs-scanner-ir.cc@145
PS3, Line 145:   auto ret = StringToDecimal4(s, len, type_precision, 
type_scale, false, result);
> Will we ever want to implement rounding in the scanner?
The plan is to not implement rounding in the scanner because it should never be 
necessary. We do not expect decimals in text files to have more digits after 
the dot than the scale of the decimal data type. If this is detected (which is 
an underflow), the scanner errors out.


http://gerrit.cloudera.org:8080/#/c/8774/3/be/src/util/string-parser.h
File be/src/util/string-parser.h:

http://gerrit.cloudera.org:8080/#/c/8774/3/be/src/util/string-parser.h@243
PS3, Line 243: // There are a maximum number of digits to the left of 
the dot. We round by
> I finally understand this comment.
Nice :)



--
To view, visit http://gerrit.cloudera.org:8080/8774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Gerrit-Change-Number: 8774
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 22 Dec 2017 00:14:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3526: update FE tests to pass on S3

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8890 )

Change subject: IMPALA-3526: update FE tests to pass on S3
..


Patch Set 2:

I agree that it's probably best to unblock first. Making it more general can be 
done in a future patch.


--
To view, visit http://gerrit.cloudera.org:8080/8890
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c8221949e76b0a0b9192e6b56c4da5eeae04141
Gerrit-Change-Number: 8890
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Dec 2017 23:39:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5478: Run TPCDS queries with decimal v2 enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8985


Change subject: IMPALA-5478: Run TPCDS queries with decimal_v2 enabled
..

IMPALA-5478: Run TPCDS queries with decimal_v2 enabled

We add new TPCDS .test files that are expected to be run with decimal_v2
enabled. The new expected results were generated using Impala and I
inspected them manually.

Change-Id: Ib867c51a521ec4a087bc127d99aee4b95ba97733
---
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q1.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q10a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q11.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q12.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q13.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q15.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q16.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q17.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q19.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q2.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q20.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q21.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q25.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q29.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q3.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q32.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q33.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q34.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q37.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-1.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-2.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q4.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q40.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q41.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q42.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q43.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q46.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q50.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q52.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q53.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q54.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q55.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q56.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q6.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q60.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q61.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q62.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q64.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q65.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q68.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q69.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q7.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q70a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q71.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q72.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q73.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q74.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q75.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q76.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q78.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q79.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q8.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q80a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q81.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q82.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q84.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q86a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q88.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q91.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q92.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q94.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q95.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q96.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q97.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q98.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q99.test
A tests/query_test/test_tpcds_decimal_v2_queries.py
73 files changed, 14,428 

[Impala-ASF-CR] IMPALA-5014: Part 2: Round when casting decimal to timestamp

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8969


Change subject: IMPALA-5014: Part 2: Round when casting decimal to timestamp
..

IMPALA-5014: Part 2: Round when casting decimal to timestamp

When there are too many digits to the right of the dot in a decimal, we
would always truncate when casting to timestamp. In this patch we change
the behavior to round instead of truncating when decimal_v2 is enabled.

Testing:
- Added some EE tests, ran BE tests on my machine.

Change-Id: I8fb3a7d976ab980b8572d7e9524850572bad57da
---
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/decimal-operators.h
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 65 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/8969/1
--
To view, visit http://gerrit.cloudera.org:8080/8969
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8fb3a7d976ab980b8572d7e9524850572bad57da
Gerrit-Change-Number: 8969
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-5014: Part 2: Round when casting decimal to timestamp

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8969 )

Change subject: IMPALA-5014: Part 2: Round when casting decimal to timestamp
..

IMPALA-5014: Part 2: Round when casting decimal to timestamp

When there are too many digits to the right of the dot in a decimal, we
would always truncate when casting to timestamp. In this patch we change
the behavior to round instead of truncating when decimal_v2 is enabled.

Testing:
- Added some EE tests, ran BE tests on my machine.

Change-Id: I8fb3a7d976ab980b8572d7e9524850572bad57da
---
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/decimal-operators.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.h
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
5 files changed, 83 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/8969/2
--
To view, visit http://gerrit.cloudera.org:8080/8969
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fb3a7d976ab980b8572d7e9524850572bad57da
Gerrit-Change-Number: 8969
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-5478: Run TPCDS queries with decimal v2 enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8985 )

Change subject: IMPALA-5478: Run TPCDS queries with decimal_v2 enabled
..

IMPALA-5478: Run TPCDS queries with decimal_v2 enabled

We add new TPCDS .test files that are expected to be run with decimal_v2
enabled. The new expected results were generated using Impala and I
inspected them manually.

Change-Id: Ib867c51a521ec4a087bc127d99aee4b95ba97733
---
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q1.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q10a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q11.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q12.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q13.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q15.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q16.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q17.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q19.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q2.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q20.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q21.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q25.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q29.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q3.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q32.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q33.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q34.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q37.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-1.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-2.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q4.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q40.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q41.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q42.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q43.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q46.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q50.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q52.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q53.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q54.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q55.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q56.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q6.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q60.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q61.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q62.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q64.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q65.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q68.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q69.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q7.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q70a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q71.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q72.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q73.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q74.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q75.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q76.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q78.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q79.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q8.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q80a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q81.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q82.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q84.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q86a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q88.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q91.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q92.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q94.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q95.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q96.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q97.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q98.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q99.test
M tests/common/test_vector.py
M tests/query_test/test_tpcds_queries.py
74 files 

[Impala-ASF-CR] IMPALA-6231: Implement decimal v2 fuzz test

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8898 )

Change subject: IMPALA-6231: Implement decimal_v2 fuzz test
..

IMPALA-6231: Implement decimal_v2 fuzz test

Implement a test that generates random decimal numbers in the pytest
framework, performs a random mathemtaical operation in Impala and
verifies that the result is correct by doing the same operating using
the Python decimal module. We try to generate not only completely random
decimal numbers, but also numbers that have interesting properties, such
as the number being a power of two.

Change-Id: I4328125de5c583ec8ead1f78d9a08703b18b2d85
---
A tests/query_test/test_decimal_fuzz.py
1 file changed, 248 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/8898/3
--
To view, visit http://gerrit.cloudera.org:8080/8898
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4328125de5c583ec8ead1f78d9a08703b18b2d85
Gerrit-Change-Number: 8898
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6388: Fix the Union node number of hosts estimation

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9017 )

Change subject: IMPALA-6388: Fix the Union node number of hosts estimation
..


Patch Set 5:

bump up*


--
To view, visit http://gerrit.cloudera.org:8080/9017
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51e1ecca8dbc84b2b5a72708667b2799d00279f0
Gerrit-Change-Number: 9017
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 16 Jan 2018 21:42:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6388: Fix the Union node number of hosts estimation

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9017 )

Change subject: IMPALA-6388: Fix the Union node number of hosts estimation
..


Patch Set 5:

I pump up the max row size in the latest patch and this fixes the problem on my 
local machine.


--
To view, visit http://gerrit.cloudera.org:8080/9017
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51e1ecca8dbc84b2b5a72708667b2799d00279f0
Gerrit-Change-Number: 9017
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 16 Jan 2018 21:42:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6388: Fix the Union node number of hosts estimation

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9017 )

Change subject: IMPALA-6388: Fix the Union node number of hosts estimation
..

IMPALA-6388: Fix the Union node number of hosts estimation

Before this patch, we would estimate the number of hosts for the union
node by looking only at the first union operand. This is obviously
incorrect and lead us to underestimate the value.

We fix the problem by setting the estimate to be the maximum of its
children.

Testing:
- Added a planner test that reproduces the issue

Change-Id: I51e1ecca8dbc84b2b5a72708667b2799d00279f0
---
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/tpch/queries/insert_parquet.test
M testdata/workloads/tpch/queries/tpch-aggregations.test
4 files changed, 93 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/17/9017/5
--
To view, visit http://gerrit.cloudera.org:8080/9017
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51e1ecca8dbc84b2b5a72708667b2799d00279f0
Gerrit-Change-Number: 9017
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9062


Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests. We also remove the Decimal_V2
TPCDS workload. This patch should only be included in the Impala 3.0
branch.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M testdata/workloads/functional-query/queries/QueryTest/decimal.test
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M testdata/workloads/functional-query/queries/QueryTest/java-udf.test
M 
testdata/workloads/functional-query/queries/QueryTest/libs_with_same_filenames.test
M testdata/workloads/functional-query/queries/QueryTest/load-java-udfs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M 
testdata/workloads/functional-query/queries/QueryTest/udf-codegen-required.test
M testdata/workloads/functional-query/queries/QueryTest/udf-init-close.test
M 
testdata/workloads/functional-query/queries/QueryTest/udf-non-deterministic.test
M testdata/workloads/functional-query/queries/QueryTest/udf.test
M 
testdata/workloads/functional-query/queries/QueryTest/utc-timestamp-functions.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q1.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q10a.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q11.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q12.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q13.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q15.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q16.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q17.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18a.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q19.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q2.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q20.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q21.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q25.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q29.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q3.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q32.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q33.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q34.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q37.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-1.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-2.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q4.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q40.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q41.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q42.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q43.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q46.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q50.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51a.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q52.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q53.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q54.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q55.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q56.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q6.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q60.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q61.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q62.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q64.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q65.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67a.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q68.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q69.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q7.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q70a.test
D testdata/workloads/tpcds/queries/tpcds-decimal_v2-q71.test
D 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 20:12:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_mem_usage_scaling.py
M tests/query_test/test_tpcds_queries.py
23 files changed, 235 insertions(+), 139 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/9062/6
--
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: newpatchset
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 6
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_tpcds_queries.py
22 files changed, 212 insertions(+), 144 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/9062/4
--
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: newpatchset
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9062 )

Change subject: IMPALA-4924: Enable Decimal V2 by default
..

IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
---
M be/src/exprs/expr-test.cc
M common/thrift/ImpalaInternalService.thrift
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
M testdata/workloads/functional-query/queries/QueryTest/uda.test
M testdata/workloads/functional-query/queries/QueryTest/values.test
M testdata/workloads/tpch/queries/tpch-q1.test
M testdata/workloads/tpch/queries/tpch-q14.test
M testdata/workloads/tpch/queries/tpch-q17.test
M testdata/workloads/tpch/queries/tpch-q8.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
M testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
M tests/hs2/test_hs2.py
M tests/query_test/test_aggregation.py
M tests/query_test/test_decimal_casting.py
M tests/query_test/test_tpcds_queries.py
22 files changed, 231 insertions(+), 140 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/9062/5
--
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: newpatchset
Gerrit-Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Gerrit-Change-Number: 9062
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

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 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 09:01:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4924: Enable Decimal V2 by default

[Taras Bobrovytsky (Code Review)]

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 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9062/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9062/3/be/src/exprs/expr-test.cc@7618
PS3, Line 7618: TEST_F(ExprTest, DecimalOverflowCasts) {
> Sorry I missed these the first time around. I think we should duplicate thi
Done


http://gerrit.cloudera.org:8080/#/c/9062/1/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/9062/1/tests/hs2/test_hs2.py@a443
PS1, Line 443:
> Did you miss this one?
Yes, I missed this one. Fixed now



--
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: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Fri, 19 Jan 2018 06:23:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6410: Tool to cherrypick changes across branches.

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9045 )

Change subject: IMPALA-6410: Tool to cherrypick changes across branches.
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9045/2/bin/compare_branches.py
File bin/compare_branches.py:

http://gerrit.cloudera.org:8080/#/c/9045/2/bin/compare_branches.py@21
PS2, Line 21: # [ { "source": "master", "target": "2.x", "commits": [ 
"", 
"" ] } ]
long line


http://gerrit.cloudera.org:8080/#/c/9045/2/bin/compare_branches.py@30
PS2, Line 30: from pprint import pformat
We should have "import..." grouped together and sorted, followed by "from ... 
import ..." statements also sorted


http://gerrit.cloudera.org:8080/#/c/9045/2/bin/compare_branches.py@179
PS2, Line 179: print "Cherrypicking %d changes." % 
(len(cherry_pick_hashes),)
The main() function seems long. Do you think it would make sense to separate 
the cherry picking into its own function?



--
To view, visit http://gerrit.cloudera.org:8080/9045
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6120ec2d6e914a1e5fda568178b32aafda8722a9
Gerrit-Change-Number: 9045
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Wed, 17 Jan 2018 20:35:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Bumping version to 3.0.

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9044 )

Change subject: Bumping version to 3.0.
..


Patch Set 2: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/9044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id39f9648cb9b40b67b1029fa8c4132cd04c1d0c6
Gerrit-Change-Number: 9044
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Wed, 17 Jan 2018 20:35:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5478: Run TPCDS queries with decimal v2 enabled

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/8985 )

Change subject: IMPALA-5478: Run TPCDS queries with decimal_v2 enabled
..

IMPALA-5478: Run TPCDS queries with decimal_v2 enabled

We add new TPCDS .test files that are expected to be run with decimal_v2
enabled. The new expected results were generated using Impala and I
inspected them manually.

Change-Id: Ib867c51a521ec4a087bc127d99aee4b95ba97733
---
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q1.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q10a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q11.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q12.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q13.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q15.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q16.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q17.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q19.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q2.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q20.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q21.test
R testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q25.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q29.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q3.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q32.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q33.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q34.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q37.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-1.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q39-2.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q4.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q40.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q41.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q42.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q43.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q46.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q50.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q51a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q52.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q53.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q54.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q55.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q56.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q6.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q60.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q61.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q62.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q64.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q65.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q68.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q69.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q7.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q70a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q71.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q72.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q73.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q74.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q75.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q76.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q78.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q79.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q8.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q80a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q81.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q82.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q84.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q86a.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q88.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q91.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q92.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q94.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q95.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q96.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q97.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q98.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q99.test
M tests/common/test_vector.py
M tests/query_test/test_tpcds_queries.py
74 files