[Impala-ASF-CR] IMPALA-3436: Return a decimal when rounding a double
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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow
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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 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-NagyGerrit-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 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 BobrovytskyGerrit-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 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-NagyGerrit-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 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 BobrovytskyGerrit-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 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 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: anujphadkeGerrit-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 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-NagyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 VigGerrit-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 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 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 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-NagyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-6522: [DOCS] Document Decimal V2
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 RodoniGerrit-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 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 ZeyligerGerrit-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 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 RodoniGerrit-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 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 ZeyligerGerrit-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 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 ZeyligerGerrit-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 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 RodoniGerrit-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 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 WangGerrit-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 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: anujphadkeGerrit-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 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 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 WangGerrit-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 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 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 WangGerrit-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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 BobrovytskyGerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-6949: Add the option to start the minicluster with EC enabled
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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-6300: Fix decimal modulo overflow
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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 ErcegovacGerrit-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 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 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 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 BobrovytskyGerrit-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 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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 BobrovytskyGerrit-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 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 ZeyligerGerrit-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 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 ZeyligerGerrit-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 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