[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 Currently, codegen supports converting type attributes (e.g. decimal type's precision and scale, type's size) obtained via calls to FunctionContextImpl::GetFnAttr() (previously Expr::GetConstantInt()) in cross-compiled code to runtime constants. This change extends this support for the query option DECIMAL_V2. To test this change, this change also handles a subset of IMPALA-2020: casting between decimal values is updated to support rounding (instead of truncation) when decimal_v2 is true. This change also cleans up the existing code by moving the codegen logic Expr::InlineConstant() to the codegen module and the type related logic in Expr::GetConstantInt() to FunctionContextImpl. Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Reviewed-on: http://gerrit.cloudera.org:8080/5950 Reviewed-by: Michael Ho Tested-by: Impala Public Jenkins --- M be/src/benchmarks/hash-benchmark.cc M be/src/codegen/llvm-codegen-test.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/partitioned-aggregation-node.cc M be/src/exec/partitioned-aggregation-node.h M be/src/exprs/agg-fn-evaluator.cc M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/conditional-functions-ir.cc M be/src/exprs/decimal-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/decimal-operators.h M be/src/exprs/expr-codegen-test.cc M be/src/exprs/expr-test.cc M be/src/exprs/expr.cc M be/src/exprs/expr.h M be/src/exprs/math-functions-ir.cc M be/src/exprs/scalar-fn-call.cc M be/src/runtime/decimal-value.inline.h M be/src/runtime/lib-cache.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/service/frontend.cc M be/src/udf/udf-internal.h M be/src/udf/udf-test-harness.cc M be/src/udf/udf-test-harness.h M be/src/udf/udf.cc M testdata/workloads/functional-query/queries/QueryTest/decimal.test M tests/query_test/test_decimal_casting.py 29 files changed, 645 insertions(+), 491 deletions(-) Approvals: Impala Public Jenkins: Verified Michael Ho: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4810: fix incorrect expr-test decimal types
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-4810: fix incorrect expr-test decimal types .. IMPALA-4810: fix incorrect expr-test decimal types Many of the types for the decimal round/truncate and related tests are incorrect. This was never caught because the type was only used by the string parser, and larger types would work. The change for IMPALA-4370 adds validation of the expected type, so fix these. Change-Id: I1e750fc01ab64ff27182670d8e823c012743804b Reviewed-on: http://gerrit.cloudera.org:8080/5959 Reviewed-by: Dan Hecht Tested-by: Impala Public Jenkins --- M be/src/exprs/expr-test.cc M be/src/runtime/types.cc 2 files changed, 101 insertions(+), 87 deletions(-) Approvals: Impala Public Jenkins: Verified Dan Hecht: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1e750fc01ab64ff27182670d8e823c012743804b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4810: fix incorrect expr-test decimal types
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4810: fix incorrect expr-test decimal types .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e750fc01ab64ff27182670d8e823c012743804b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 4: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/263/ -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Michael Ho has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 4: Code-Review+2 Carry +2 forward. -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Michael Ho has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 4: Added decimal_v2 as a test dimension for decimal_casting.py. It's disabled for now as we cannot easily test decimal to decimal casting without first fixing 'string/int/double' to decimal casting. Also added decimal to decimal casting to the test case now as it was missing before. -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5950 to look at the new patch set (#4). Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 Currently, codegen supports converting type attributes (e.g. decimal type's precision and scale, type's size) obtained via calls to FunctionContextImpl::GetFnAttr() (previously Expr::GetConstantInt()) in cross-compiled code to runtime constants. This change extends this support for the query option DECIMAL_V2. To test this change, this change also handles a subset of IMPALA-2020: casting between decimal values is updated to support rounding (instead of truncation) when decimal_v2 is true. This change also cleans up the existing code by moving the codegen logic Expr::InlineConstant() to the codegen module and the type related logic in Expr::GetConstantInt() to FunctionContextImpl. Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 --- M be/src/benchmarks/hash-benchmark.cc M be/src/codegen/llvm-codegen-test.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/partitioned-aggregation-node.cc M be/src/exec/partitioned-aggregation-node.h M be/src/exprs/agg-fn-evaluator.cc M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/conditional-functions-ir.cc M be/src/exprs/decimal-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/decimal-operators.h M be/src/exprs/expr-codegen-test.cc M be/src/exprs/expr-test.cc M be/src/exprs/expr.cc M be/src/exprs/expr.h M be/src/exprs/math-functions-ir.cc M be/src/exprs/scalar-fn-call.cc M be/src/runtime/decimal-value.inline.h M be/src/runtime/lib-cache.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/service/frontend.cc M be/src/udf/udf-internal.h M be/src/udf/udf-test-harness.cc M be/src/udf/udf-test-harness.h M be/src/udf/udf.cc M testdata/workloads/functional-query/queries/QueryTest/decimal.test M tests/query_test/test_decimal_casting.py 29 files changed, 645 insertions(+), 491 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/5950/4 -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5950 to look at the new patch set (#3). Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 Currently, codegen supports converting type attributes (e.g. decimal type's precision and scale, type's size) obtained via calls to FunctionContextImpl::GetFnAttr() (previously Expr::GetConstantInt()) in cross-compiled code to runtime constants. This change extends this support for the query option DECIMAL_V2. To test this change, this change also handles a subset of IMPALA-2020: casting between decimal values is updated to support rounding (instead of truncation) when decimal_v2 is true. This change also cleans up the existing code by moving the codegen logic Expr::InlineConstant() to the codegen module and the type related logic in Expr::GetConstantInt() to FunctionContextImpl. Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 --- M be/src/benchmarks/hash-benchmark.cc M be/src/codegen/llvm-codegen-test.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/partitioned-aggregation-node.cc M be/src/exec/partitioned-aggregation-node.h M be/src/exprs/agg-fn-evaluator.cc M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/conditional-functions-ir.cc M be/src/exprs/decimal-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/decimal-operators.h M be/src/exprs/expr-codegen-test.cc M be/src/exprs/expr-test.cc M be/src/exprs/expr.cc M be/src/exprs/expr.h M be/src/exprs/math-functions-ir.cc M be/src/exprs/scalar-fn-call.cc M be/src/runtime/decimal-value.inline.h M be/src/runtime/lib-cache.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/service/frontend.cc M be/src/udf/udf-internal.h M be/src/udf/udf-test-harness.cc M be/src/udf/udf-test-harness.h M be/src/udf/udf.cc M testdata/workloads/functional-query/queries/QueryTest/decimal.test M tests/query_test/test_decimal_casting.py 29 files changed, 640 insertions(+), 488 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/5950/3 -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-2020: Add rounding for decimal casts
Zach Amsden has uploaded a new patch set (#5). Change subject: IMPALA-2020: Add rounding for decimal casts .. IMPALA-2020: Add rounding for decimal casts This change adds support for DECIMAL_V2 rounding behavior for both DECIMAL to INT and DOUBLE to DECIMAL casts. Still writing tests, but this is ready for human eyes to look at. Testing: In progress. Rouding to int works as expected, rounding from float seems to work as well. Expect a full test suite for both modes and all edge cases to be covered. Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810 --- M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/expr.h M be/src/exprs/literal.cc M be/src/runtime/decimal-test.cc M be/src/runtime/decimal-value.h M be/src/runtime/decimal-value.inline.h M be/src/udf/udf.h 7 files changed, 149 insertions(+), 71 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/5951/5 -- To view, visit http://gerrit.cloudera.org:8080/5951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I543d4d428d7240e0f710d67973867162f2fcabc8 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI .. IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI This commit adds heap and non-heap memory usage of the embedded JVM in the memory metrics and exposes these metrics in /memz web page of the impalad and catalog web UI. Change-Id: I543d4d428d7240e0f710d67973867162f2fcabc8 Reviewed-on: http://gerrit.cloudera.org:8080/5909 Reviewed-by: Dimitris Tsirogiannis Tested-by: Impala Public Jenkins --- M be/src/util/default-path-handlers.cc M fe/src/main/java/org/apache/impala/common/JniUtil.java M www/memz.tmpl 3 files changed, 110 insertions(+), 8 deletions(-) Approvals: Impala Public Jenkins: Verified Dimitris Tsirogiannis: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I543d4d428d7240e0f710d67973867162f2fcabc8 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4897: AnalysisException: specified cache pool does not exist .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5972/1/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS1, Line 258: if IS_HDFS: : self.run_test_case('QueryTest/alter-table-hdfs-caching', vector, : use_db=unique_database, multiple_impalad=self._use_multiple_impalad(vector)) > I think it may be best to put it in test_hdfs_caching.py? What do you think I thought about that but the goal of this test is to test an ALTER TABLE functionality and the hdfs caching part is not the core of the test, which is why I left it here instead. -- To view, visit http://gerrit.cloudera.org:8080/5972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-4897: AnalysisException: specified cache pool does not exist .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5972/1/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS1, Line 258: if IS_HDFS: : self.run_test_case('QueryTest/alter-table-hdfs-caching', vector, : use_db=unique_database, multiple_impalad=self._use_multiple_impalad(vector)) I think it may be best to put it in test_hdfs_caching.py? What do you think? -- To view, visit http://gerrit.cloudera.org:8080/5972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4897: AnalysisException: specified cache pool does not exist
Sailesh Mukil has uploaded a new change for review. http://gerrit.cloudera.org:8080/5972 Change subject: IMPALA-4897: AnalysisException: specified cache pool does not exist .. IMPALA-4897: AnalysisException: specified cache pool does not exist A few tests were added with IMPALA-1670 that made use of HDFS caching. This patch moves these tests to a new file and only executes them when the default filesystem is HDFS. Also added a testcase for testing alter table ADD multiple PARTITIONS for non-HDFS filesystems. Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a --- A testdata/workloads/functional-query/queries/QueryTest/alter-table-hdfs-caching.test M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M tests/metadata/test_ddl.py 3 files changed, 123 insertions(+), 65 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/5972/1 -- To view, visit http://gerrit.cloudera.org:8080/5972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iefe61556bc28ae320f3f41fdc930d37b258d970a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil
[Impala-ASF-CR] IMPALA-4905: Reduce coordinator lock contention in RPC handler
Henry Robinson has posted comments on this change. Change subject: IMPALA-4905: Reduce coordinator lock contention in RPC handler .. Patch Set 1: (1 comment) Changed how the logging works - it didn't quite work in the previous patch because WaitForAllInstance() isn't called until late on in query execution. http://gerrit.cloudera.org:8080/#/c/5971/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: PS1, Line 829: for (const InstanceState* state: fragment_instance_states_) { : files_to_move.insert( : state->insert_status().files_to_move.begin(), : state->insert_status().files_to_move.end()); : : for (const PartitionStatusMap::value_type& partition: : state->insert_status().per_partition_status) { : TInsertPartitionStatus* status = &(per_partition_status_[partition.first]); : status->__set_num_modified_rows( : status->num_modified_rows + partition.second.num_modified_rows); : status->__set_kudu_latest_observed_ts(std::max( : partition.second.kudu_latest_observed_ts, status->kudu_latest_observed_ts)); : status->__set_id(partition.second.id); : status->__set_partition_base_dir(partition.second.partition_base_dir); : : if (partition.second.__isset.stats) { : if (!status->__isset.stats) status->__set_stats(TInsertStats()); : DataSink::MergeDmlStats(partition.second.stats, &status->stats); : } : } : } > I expect that large INSERT queries will take a while longer to run because I don't think it's going to be excessive, even though this is an N^2 loop (the metadata update times usually dominate). Worth checking to see if it makes a difference though. -- To view, visit http://gerrit.cloudera.org:8080/5971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4859: Push down IS NULL / IS NOT NULL to Kudu
Alex Behm has posted comments on this change. Change subject: IMPALA-4859: Push down IS NULL / IS NOT NULL to Kudu .. Patch Set 2: (1 comment) I think we should also add an end-to-end. In particular, I'm curious what Kudu will do when we pass it an IS NOT NULL predicate on a primary-key column or another non-nullable column. http://gerrit.cloudera.org:8080/#/c/5958/2/testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test: Line 152: predicates: CAST(date_string_col AS TINYINT) IS NULL can move this into the query above, so we have one query with all the cases? -- To view, visit http://gerrit.cloudera.org:8080/5958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9c96fec8d41f77222879c0ffdd6940b168e47e65 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4905: Reduce coordinator lock contention in RPC handler
Henry Robinson has uploaded a new patch set (#2). Change subject: IMPALA-4905: Reduce coordinator lock contention in RPC handler .. IMPALA-4905: Reduce coordinator lock contention in RPC handler Fragment instances call ReportExecStatus every few seconds, so there are often very many of those RPCs in flight. The handler in Coordinator::UpdateFragmentExecStatus() would try and take the shared coordinator lock twice, once to merge in any insert file move operations to be performed, and once to print all the other instance state's status. There was also a bug where the insert-merge branch would always be taken (and so would the lock) even if the query was not an INSERT statement. This patch refactors UpdateFragmentExecStatus() so that it never takes the coordinator lock. Instead, the insert updates are saved in the fragment instance state to be merged during finalization. The logging is now done without taking the coordinator lock unnecessarily, and also avoid taking the fragment instance state locks since done() can be read safely without synchronization on x86. Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/fragment-instance-state.cc 3 files changed, 61 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/5971/2 -- To view, visit http://gerrit.cloudera.org:8080/5971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2
Dan Hecht has posted comments on this change. Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version V2 .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/5952/6/fe/src/main/java/org/apache/impala/catalog/ScalarType.java File fe/src/main/java/org/apache/impala/catalog/ScalarType.java: PS6, Line 115: precision > Yeah I thought about adding a check for that but it wasn't entirely clear t The answer is that it can be > 38. Negative is a parser error (not an integral literal) but > 38 is an analysis error (see TypeDef.analyzeScalarType()). -- To view, visit http://gerrit.cloudera.org:8080/5952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4810: fix incorrect expr-test decimal types
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4810: fix incorrect expr-test decimal types .. Patch Set 3: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/262/ -- To view, visit http://gerrit.cloudera.org:8080/5959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e750fc01ab64ff27182670d8e823c012743804b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4810: fix incorrect expr-test decimal types
Dan Hecht has posted comments on this change. Change subject: IMPALA-4810: fix incorrect expr-test decimal types .. Patch Set 3: Code-Review+2 Rebase -- To view, visit http://gerrit.cloudera.org:8080/5959 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e750fc01ab64ff27182670d8e823c012743804b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4905: Reduce coordinator lock contention in RPC handler
Henry Robinson has posted comments on this change. Change subject: IMPALA-4905: Reduce coordinator lock contention in RPC handler .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5971/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: PS1, Line 1562: num_remaining_fragment_instances_.Load(); > Just print "num_remaining" ? probably should print num_remaining for consistency but - on x86, Load() translates to a mov plus a compiler barrier, because loads on x86 have acquire semantics. So the bus doesn't get locked. -- To view, visit http://gerrit.cloudera.org:8080/5971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4905: Reduce coordinator lock contention in RPC handler
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4905: Reduce coordinator lock contention in RPC handler .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5971/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: PS1, Line 829: for (const InstanceState* state: fragment_instance_states_) { : files_to_move.insert( : state->insert_status().files_to_move.begin(), : state->insert_status().files_to_move.end()); : : for (const PartitionStatusMap::value_type& partition: : state->insert_status().per_partition_status) { : TInsertPartitionStatus* status = &(per_partition_status_[partition.first]); : status->__set_num_modified_rows( : status->num_modified_rows + partition.second.num_modified_rows); : status->__set_kudu_latest_observed_ts(std::max( : partition.second.kudu_latest_observed_ts, status->kudu_latest_observed_ts)); : status->__set_id(partition.second.id); : status->__set_partition_base_dir(partition.second.partition_base_dir); : : if (partition.second.__isset.stats) { : if (!status->__isset.stats) status->__set_stats(TInsertStats()); : DataSink::MergeDmlStats(partition.second.stats, &status->stats); : } : } : } I expect that large INSERT queries will take a while longer to run because of this change. Are we okay with that? PS1, Line 1555: DCHECK_GT(num_remaining_fragment_instances_.Load(), 0); You can move this DCHECK to L1557 and convert it to: DCHECK_GE(num_remaining, 0); PS1, Line 1562: num_remaining_fragment_instances_.Load(); Just print "num_remaining" ? It's guaranteed that someone will print the most updated value if they race between 1556 and here. I don't see any reason to potentially lock the bus again for this. -- To view, visit http://gerrit.cloudera.org:8080/5971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4859: Push down IS NULL / IS NOT NULL to Kudu
Joe McDonnell has uploaded a new patch set (#2). Change subject: IMPALA-4859: Push down IS NULL / IS NOT NULL to Kudu .. IMPALA-4859: Push down IS NULL / IS NOT NULL to Kudu This detects IS NULL / IS NOT NULL and creates a Kudu predicate to push this to Kudu. Since the KuduPredicate.newIsNullPredicate function is only available in a newer version of the Kudu Java client, that client version is bumped to 1.3.0. Change-Id: I9c96fec8d41f77222879c0ffdd6940b168e47e65 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test 4 files changed, 60 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/5958/2 -- To view, visit http://gerrit.cloudera.org:8080/5958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9c96fec8d41f77222879c0ffdd6940b168e47e65 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-4905: Reduce coordinator lock contention in RPC handler
Henry Robinson has uploaded a new change for review. http://gerrit.cloudera.org:8080/5971 Change subject: IMPALA-4905: Reduce coordinator lock contention in RPC handler .. IMPALA-4905: Reduce coordinator lock contention in RPC handler Fragment instances call ReportExecStatus every few seconds, so there are often very many of those RPCs in flight. The handler in Coordinator::UpdateFragmentExecStatus() would try and take the shared coordinator lock twice, once to merge in any insert file move operations to be performed, and once to print all the other instance state's status. There was also a bug where the insert-merge branch would always be taken (and so would the lock) even if the query was not an INSERT statement. This patch refactors UpdateFragmentExecStatus() so that it never takes the coordinator lock. Instead, the insert updates are saved in the fragment instance state to be merged during finalization, and the logging is now done by the main coordinator thread which is signalled by the RPC handler. Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/fragment-instance-state.cc 3 files changed, 77 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/5971/1 -- To view, visit http://gerrit.cloudera.org:8080/5971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id7599780785c4e9306711f535bf4726a247873e2 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson
[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2
Dan Hecht has uploaded a new patch set (#7). Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version V2 .. IMPALA-4370: Divide and modulo result types for DECIMAL version V2 Implement the new DECIMAL return type rules for divide and modulo expressions, active when query option DECIMAL_V2=1. See the comment in the code for more details. A couple of examples that show why new return type rules for divide are desirable. For modulo, the return types are actually equivalent, though the rules are expressed differently to have consistency with how precision fixups are handled for each version. DECIMAL Version 1: +---+ | cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) | +-+ | 0 | +---+ DECIMAL Version 2: +---+ | cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) | +-+ | 0.33| +---+ DECIMAL Version 1: +---+ | cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) | +---+ | NULL | +---+ WARNINGS: UDF WARNING: Expression overflowed, returning NULL DECIMAL Version 2: +---+ | cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) | +---+ | 10.00 | +---+ Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b --- M be/src/exprs/expr-test.cc M be/src/runtime/decimal-value.inline.h M be/src/testutil/impalad-query-executor.h M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java M fe/src/main/java/org/apache/impala/catalog/ScalarType.java M fe/src/main/java/org/apache/impala/catalog/Type.java A testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test M tests/query_test/test_decimal_queries.py 10 files changed, 325 insertions(+), 75 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/5952/7 -- To view, visit http://gerrit.cloudera.org:8080/5952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2
Dan Hecht has posted comments on this change. Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version V2 .. Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/5952/6/fe/src/main/java/org/apache/impala/catalog/ScalarType.java File fe/src/main/java/org/apache/impala/catalog/ScalarType.java: PS6, Line 115: precision > Can precision be > MAX_PRECISION in this case ? Yeah I thought about adding a check for that but it wasn't entirely clear to me so I left it alone for now. PS6, Line 138: ClipPrecScale > May help to clarify that how this function and the *AdjustPrecScale() are r I don't think it will be as simple as use one when v1 is enabled and the other when v2 is enabled. I think there are some cases in v2 where we still may want to clip. Line 148:* preserve at least MIN_ADJUSTED_SCALE for scale (unless the desired scale was less). > MIN_ADJUSTED_SCALE for scale (unless ...) Done http://gerrit.cloudera.org:8080/#/c/5952/6/fe/src/main/java/org/apache/impala/catalog/Type.java File fe/src/main/java/org/apache/impala/catalog/Type.java: PS6, Line 65: > nit: blank space Done -- To view, visit http://gerrit.cloudera.org:8080/5952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] Revert "IMPALA-4829: Change default Kudu read behavior for "RYW""
Matthew Jacobs has uploaded a new change for review. http://gerrit.cloudera.org:8080/5970 Change subject: Revert "IMPALA-4829: Change default Kudu read behavior for "RYW"" .. Revert "IMPALA-4829: Change default Kudu read behavior for "RYW"" Reverting until we have a fix for KUDU-1869: Scans do not work with hybrid time disabled and snapshot reads enabled This reverts commit 32ff959814646458a34278500bd01fc7741951ce. Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262 --- M be/src/exec/kudu-scanner.cc 1 file changed, 7 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/5970/1 -- To view, visit http://gerrit.cloudera.org:8080/5970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I995dec543946c9e0f79bc5b7e82568060a9d8262 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs
[Impala-ASF-CR] IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI .. Patch Set 4: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/261/ -- To view, visit http://gerrit.cloudera.org:8080/5909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I543d4d428d7240e0f710d67973867162f2fcabc8 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI
Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-4884: Add JVM heap and non-heap usage in metrics and UI .. Patch Set 4: Code-Review+2 Rebase, keep Dan's +2 -- To view, visit http://gerrit.cloudera.org:8080/5909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I543d4d428d7240e0f710d67973867162f2fcabc8 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] Fix merge conflict
Impala Public Jenkins has posted comments on this change. Change subject: Fix merge conflict .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] Fix merge conflict
Impala Public Jenkins has submitted this change and it was merged. Change subject: Fix merge conflict .. Fix merge conflict Commits fcc2d81 and 1335af3 conflicted. Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Reviewed-on: http://gerrit.cloudera.org:8080/5967 Reviewed-by: Bharath Vissapragada Reviewed-by: Dimitris Tsirogiannis Tested-by: Impala Public Jenkins --- M common/thrift/generate_error_codes.py 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Impala Public Jenkins: Verified Bharath Vissapragada: Looks good to me, but someone else must approve Dimitris Tsirogiannis: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-4810: Make DECIMAL expr-test cases table driven
Dan Hecht has posted comments on this change. Change subject: IMPALA-4810: Make DECIMAL expr-test cases table driven .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/5933/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 1288: {{ false, 2230, 5, 3 }, { false, 2230, 5, 3 }} }, > but how about changing it so if the result is the same in both versions, yo yeah, i plan to do that before converting more cases but haven't had time yet. -- To view, visit http://gerrit.cloudera.org:8080/5933 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieffb2573de46bba64d1b4e8caf15cc0238a84ea1 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Michael Ho has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 2: Verified there is no perf regression with decimal_v2=false using following query: select sum(cast(l_quantity as DECIMAL(12,1)) + cast(l_extendedprice as DECIMAL(12,1)) + cast(l_discount as DECIMAL(12,1)) + cast(l_tax as DECIMAL(12,1))) from tpch20_parquet.lineitem; Avg: 2.23s With decimal_v2 to true, the query slowed down quite a bit: Avg: 4.53s Verified that the optimized IR already had constant propagated. -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4546: Fix Moscow timezone conversion after 2014
Taras Bobrovytsky has uploaded a new change for review. http://gerrit.cloudera.org:8080/5969 Change subject: IMPALA-4546: Fix Moscow timezone conversion after 2014 .. IMPALA-4546: Fix Moscow timezone conversion after 2014 In 2014 Moscow timezone rules changed from UTC+3 with no DST to UTC+4 with no DST. A special case has been added to timestamp functions to handle this. Testing: Added BE Expr tests. Change-Id: Id6e3f2c9f6ba29749a26bc1087e664637bc02528 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions.cc M be/src/exprs/timezone_db.cc M be/src/exprs/timezone_db.h 4 files changed, 107 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/5969/1 -- To view, visit http://gerrit.cloudera.org:8080/5969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id6e3f2c9f6ba29749a26bc1087e664637bc02528 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/5840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read .. Patch Set 6: Code-Review+1 carrying Sailesh's +1 -- To view, visit http://gerrit.cloudera.org:8080/5840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/5840/5//COMMIT_MSG Commit Message: PS5, Line 22: Kudu scanner > nit: I got confused thinking it was our KuduScanner. It's happening on the Done -- To view, visit http://gerrit.cloudera.org:8080/5840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Hello Sailesh Mukil, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5840 to look at the new patch set (#6). Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read .. IMPALA-4828: Alter Kudu schema outside Impala may crash on read Creating a table in Impala, changing the column schema outside of Impala, and then reading again in Impala may result in a crash. Impala may attempt to dereference pointers that aren't there. This happens if a string column is dropped and then a new, non string column is added with the old string column's name. The Kudu scan token contains the projection schema, and that is validated when opening the Kudu scanner, but the issue is that during planning, Impala assumes the types of columns haven't changed when creating the scan tokens. This is fixed by adding a check when creating the scan token, and failing the query if the column types changed. Impala then relies on the Kudu client to properly validate that the underlying schema is still represented by the scan token, and that deserialization will fail if it no longer matches. A test case was added for this particular crash scenario, which now fails during planning as expected. This does not attempt to validate the Kudu client validation at deserialization time, though that would be valuable coverage to add in the future. Columns being removed don't produce a crash; the query fails gracefully. A test was added for this case. Columns being added should not affect this scenario, but a test was added anyway. Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a --- M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java M tests/query_test/test_kudu.py 2 files changed, 116 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/5840/6 -- To view, visit http://gerrit.cloudera.org:8080/5840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-4828: Alter Kudu schema outside Impala may crash on read
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4828: Alter Kudu schema outside Impala may crash on read .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/5840/5//COMMIT_MSG Commit Message: PS5, Line 22: Kudu scanner nit: I got confused thinking it was our KuduScanner. It's happening on the Kudu side right? If so, maybe change this to "Kudu's client scanner"? -- To view, visit http://gerrit.cloudera.org:8080/5840 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d43f5bb9811e728ad592933066d006c8fb4553a Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4370: Divide and modulo result types for DECIMAL version V2
Michael Ho has posted comments on this change. Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version V2 .. Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/5952/6/fe/src/main/java/org/apache/impala/catalog/ScalarType.java File fe/src/main/java/org/apache/impala/catalog/ScalarType.java: PS6, Line 115: precision Can precision be > MAX_PRECISION in this case ? PS6, Line 138: ClipPrecScale May help to clarify that how this function and the *AdjustPrecScale() are related to decimal_v1 vs decimal_v2. Otherwise, it may be hard to tell which one to use. Line 148:* preserve at least MIN_ADJUSTED_SCALE (unless the desired scale was less). MIN_ADJUSTED_SCALE for scale (unless ...) http://gerrit.cloudera.org:8080/#/c/5952/6/fe/src/main/java/org/apache/impala/catalog/Type.java File fe/src/main/java/org/apache/impala/catalog/Type.java: PS6, Line 65: nit: blank space -- To view, visit http://gerrit.cloudera.org:8080/5952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic
Jim Apple has posted comments on this change. Change subject: IMPALA-3410 [DOCS] Rework Impala authentication topics to be generic .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/5962/2/docs/topics/impala_kerberos.xml File docs/topics/impala_kerberos.xml: PS2, Line 46: KDC Not defined yet Line 50: Can this be removed? http://gerrit.cloudera.org:8080/#/c/5962/2/docs/topics/impala_ldap.xml File docs/topics/impala_ldap.xml: Line 189: Specify the option on the impalad command line. This paragraph is confusing now. -- To view, visit http://gerrit.cloudera.org:8080/5962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I639a55eb43555cf074c26d23b5c72f778073231c Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Ambreen Kazi Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Laurel Hale Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3411 [DOCS] Rework Impala governance topics to be generic.
Jim Apple has posted comments on this change. Change subject: IMPALA-3411 [DOCS] Rework Impala governance topics to be generic. .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5957/1//COMMIT_MSG Commit Message: Line 12: to use cluster management software with a focus on governance. Why not always remove them? http://gerrit.cloudera.org:8080/#/c/5957/1/docs/topics/impala_lineage.xml File docs/topics/impala_lineage.xml: PS1, Line 60: Use a cluster manager with enhanced governance capabilities to transform : lineage data generated by Impala into graphs for easy visualization. This feels off-topic to me. -- To view, visit http://gerrit.cloudera.org:8080/5957 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I192bc2d1de89e55418c045d1a0e5433cf02cf782 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Ambreen Kazi Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: John Russell Gerrit-Reviewer: Laurel Hale Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4822: Implement dynamic log level changes
Henry Robinson has posted comments on this change. Change subject: IMPALA-4822: Implement dynamic log level changes .. Patch Set 12: > I'm not sure the extra complexity of the UI is worth it. Without the UI, we'd have to remember how to construct the URLs and what the arguments are. The only complexity the UI adds is log_level.tmpl which handles presentation of the returned data. Everything else would stay roughly the same if we had a command-line-only interface. -- To view, visit http://gerrit.cloudera.org:8080/5792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I588418e9bcb0b66d33138baf96207a5a35bfbd63 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Dan Hecht has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 2: Code-Review+2 (2 comments) Please take a look at whether it makes sense to add coverage in test_decimal_casting.py before committing. Also, please see my question earlier about whether perf (for v2=false) was checked. http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 1334: {{ false, 9, 9, 0 }, { true, 0, 9, 0 }} }, > Modified the test above to show rounding of 0.12344 vs 0.12345. Is that wha Yes http://gerrit.cloudera.org:8080/#/c/5950/1/testdata/workloads/functional-query/queries/QueryTest/decimal.test File testdata/workloads/functional-query/queries/QueryTest/decimal.test: Line 469: RESULTS > The query above was run with v2=false. Did you mean something else ? Yes. Somehow I didn't see that. -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Dan Hecht has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 1: We also have test_decimal_casting.py. I haven't looked at it in detail but maybe there's more coverage you can get there (or adjustments you need to make for rounding). -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-HasComments: No
[Impala-ASF-CR] Fix merge conflict
Dimitris Tsirogiannis has posted comments on this change. Change subject: Fix merge conflict .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] Fix merge conflict
Bharath Vissapragada has posted comments on this change. Change subject: Fix merge conflict .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/5967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] Fix merge conflict
Impala Public Jenkins has posted comments on this change. Change subject: Fix merge conflict .. Patch Set 1: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/260/ -- To view, visit http://gerrit.cloudera.org:8080/5967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] Fix merge conflict
Dan Hecht has uploaded a new change for review. http://gerrit.cloudera.org:8080/5967 Change subject: Fix merge conflict .. Fix merge conflict Commits fcc2d81 and 1335af3 conflicted. Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f --- M common/thrift/generate_error_codes.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/5967/1 -- To view, visit http://gerrit.cloudera.org:8080/5967 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia5444d6b44b9aeea18f7861849513a2bde5c881f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht
[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges. .. Patch Set 1: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/5774/1/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: Line 799: if (!childFragment.refsNullableTupleId(partitionExprs)) { combine with preceding if http://gerrit.cloudera.org:8080/#/c/5774/1/fe/src/main/java/org/apache/impala/planner/PlanFragment.java File fe/src/main/java/org/apache/impala/planner/PlanFragment.java: Line 386:* Returns true if 'exprs' reference a tuple that is made nullable in this fragment. "in this fragment but not in any of its input fragments" Line 390: List groupingExprTids = Lists.newArrayList(); remove the reference to grouping, it's overly specific. -- To view, visit http://gerrit.cloudera.org:8080/5774 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4822: Implement dynamic log level changes
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4822: Implement dynamic log level changes .. Patch Set 12: > Marcel, you can make changes like you described just with a URL, > see test_web_pages.py which does just that. The UI is nice because > then you also have a way of inspecting the state of the system. > Allowing mutation without having a way to inspect the state seems > bad for usability (and esp. if we expose it to users). I didn't suggest not allowing inspection of the existing log levels, which can also be done via a url. I'm not sure the extra complexity of the UI is worth it. -- To view, visit http://gerrit.cloudera.org:8080/5792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I588418e9bcb0b66d33138baf96207a5a35bfbd63 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Marcel Kornacker Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Michael Ho has posted comments on this change. Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. Patch Set 1: (16 comments) http://gerrit.cloudera.org:8080/#/c/5950/1//COMMIT_MSG Commit Message: Line 21: > also mention that this does part of IMPALA-2020 for decimal->decimal casts. Done http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/codegen/llvm-codegen.h File be/src/codegen/llvm-codegen.h: Line 468: boost::unordered_set* symbols); > never mind, i think i saw this. it's only used in one place and it just mov Yes. This change allows CreateFrom* to be all private functions of LlvmCodeGen. http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: PS1, Line 1740: output_type > or, after seeing the full patch, i think you can just revert this to avoid Reverted and merged with Tim's patch. http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/exprs/decimal-operators-ir.cc File be/src/exprs/decimal-operators-ir.cc: PS1, Line 478: output_precision < val_precision > this gets codegen'ed away, right? Yes, it will. Line 479: abs(result.val16) >= DecimalUtil::GetScaleMultiplier(output_precision)) { > how about using RETURN_IF_OVERFLOW() so we get the warning (and later it ma Done PS1, Line 505: static_cast(is_decimal_v2) > usually we convert to bool like this: decimal_v2 != 0 Done http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/exprs/expr-codegen-test.cc File be/src/exprs/expr-codegen-test.cc: Line 310: int64_t v = 125; > what's this? Oh, it's just verifying the rounding actually occurred with decimal_v2. PS1, Line 316: 13 > and this? Comments added. http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 1334: {{ false, -9, 9, 0 }, { true, 0, 9, 0 }} }, > how about a positive/negative test where the digit is 5 (to show round-up)? Modified the test above to show rounding of 0.12344 vs 0.12345. Is that what you mean ? PS1, Line 1357: v2 > sorry about that No problem. I missed that in the review too. http://gerrit.cloudera.org:8080/#/c/5950/1/be/src/udf/udf-internal.h File be/src/udf/udf-internal.h: PS1, Line 131: attributes of the return type and argument types of a UDF/UDA > static attributes of the UDF/UDA that can be injected as constants by codeg Done PS1, Line 148: This function returns the various attributes of the return type and argument types : /// recorded in 'ctx' : /// It also returns the value of the query option decimal_v2 which dictates the behavior : /// of cast to Decimal type > How about just say Done PS1, Line 160: /// Functions which implement the UDF or UDA interface should use this function so that : /// runtime constants of the type attributes would be inlined when codegen is enabled. > this can be deleted if you use the wording above (and it's good to mention Done Line 170: /// - whether query option decimal_v2 is set (returns 0 or 1) > could just refer to ConstFnAttr. Done http://gerrit.cloudera.org:8080/#/c/5950/1/testdata/workloads/functional-query/queries/QueryTest/decimal.test File testdata/workloads/functional-query/queries/QueryTest/decimal.test: Line 469: RESULTS > how about running this query with v2=false as well so the difference is cle The query above was run with v2=false. Did you mean something else ? Line 495: > this is going to conflict with my change, which causes us to run decimal.te Yes, we should coordinate. If your change merges first, I will move it over to the new file which doesn't exist yet in trunk. -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL V2
Michael Ho has uploaded a new patch set (#2). Change subject: IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 .. IMPALA-2020, IMPALA-4809: Codegen support for DECIMAL_V2 Currently, codegen supports converting type attributes (e.g. decimal type's precision and scale, type's size) obtained via calls to FunctionContextImpl::GetFnAttr() (previously Expr::GetConstantInt()) in cross-compiled code to runtime constants. This change extends this support for the query option DECIMAL_V2. To test this change, this change also handles a subset of IMPALA-2020: casting between decimal values is updated to support rounding (instead of truncation) when decimal_v2 is true. This change also cleans up the existing code by moving the codegen logic Expr::InlineConstant() to the codegen module and the type related logic in Expr::GetConstantInt() to FunctionContextImpl. Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 --- M be/src/benchmarks/hash-benchmark.cc M be/src/codegen/llvm-codegen-test.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/partitioned-aggregation-node.cc M be/src/exec/partitioned-aggregation-node.h M be/src/exprs/agg-fn-evaluator.cc M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/conditional-functions-ir.cc M be/src/exprs/decimal-functions-ir.cc M be/src/exprs/decimal-operators-ir.cc M be/src/exprs/decimal-operators.h M be/src/exprs/expr-codegen-test.cc M be/src/exprs/expr-test.cc M be/src/exprs/expr.cc M be/src/exprs/expr.h M be/src/exprs/math-functions-ir.cc M be/src/exprs/scalar-fn-call.cc M be/src/runtime/decimal-value.inline.h M be/src/runtime/lib-cache.cc M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/service/frontend.cc M be/src/udf/udf-internal.h M be/src/udf/udf-test-harness.cc M be/src/udf/udf-test-harness.h M be/src/udf/udf.cc M common/thrift/generate_error_codes.py M testdata/workloads/functional-query/queries/QueryTest/decimal.test 29 files changed, 605 insertions(+), 463 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/5950/2 -- To view, visit http://gerrit.cloudera.org:8080/5950 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2434d240f65b81389b8a8ba027f980a0e1d1f981 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht
[Impala-ASF-CR] IMPALA-4810: Make DECIMAL expr-test cases table driven
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4810: Make DECIMAL expr-test cases table driven .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/5933/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 1288: {{ false, 2230, 5, 3 }, { false, 2230, 5, 3 }} }, > The next patch introduces actual differences, and lines up each pair of exp but how about changing it so if the result is the same in both versions, you only need to specify it once? that would help readability quite a bit. -- To view, visit http://gerrit.cloudera.org:8080/5933 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieffb2573de46bba64d1b4e8caf15cc0238a84ea1 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes