From 2c3c3b790c4087c220b849cb7d9aa48b024a9fde Mon Sep 17 00:00:00 2001 From: philo Date: Wed, 17 Aug 2022 20:30:27 +0800 Subject: [PATCH 1/3] Optimize code logic by considering input null count --- .../cpp/src/codegen/arrow_compute/ext/actions_impl.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc index ef2f7379c..f8bb530de 100644 --- a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc +++ b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc @@ -3005,7 +3005,7 @@ class SumCountAction 0 && in_->IsNull(row_id); + const bool is_null = in_->IsNull(row_id); if (!is_null) { cache_sum_[dest_group_id] += data_[row_id]; cache_count_[dest_group_id] += 1; @@ -3185,7 +3185,7 @@ class SumCountAction 0 && in_->IsNull(row_id); + const bool is_null = in_->IsNull(row_id); if (!is_null) { cache_sum_[dest_group_id] += in_->GetView(row_id); cache_count_[dest_group_id] += 1; @@ -5059,8 +5059,9 @@ class FirstPartialAction(in[0]); + int in_null_count = input_array->null_count(); for (int id = 0; id < input_array->length(); id++) { - if (input_array->IsNull(id)) { + if (in_null_count > 0 && input_array->IsNull(id)) { if (ignore_nulls_) { continue; } else { @@ -5296,8 +5297,9 @@ class FirstPartialAction(in[0]); + int in_null_count = input_array->null_count(); for (int id = 0; id < input_array->length(); id++) { - if (input_array->IsNull(id)) { + if (in_null_count > 0 && input_array->IsNull(id)) { if (ignore_nulls_) { continue; } else { From f42f3b8db3762bdaaece2a4f976d2072fcfccefe Mon Sep 17 00:00:00 2001 From: philo Date: Thu, 18 Aug 2022 13:53:48 +0800 Subject: [PATCH 2/3] Pass input list in const reference --- .../codegen/arrow_compute/ext/actions_impl.cc | 64 +++++++++---------- .../codegen/arrow_compute/ext/actions_impl.h | 4 +- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc index f8bb530de..722ce2afd 100644 --- a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc +++ b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc @@ -50,14 +50,14 @@ struct FindAccumulatorType> { using Type = arrow::DoubleType; }; -arrow::Status ActionBase::Submit(ArrayList in, int max_group_id, +arrow::Status ActionBase::Submit(const ArrayList& in, int max_group_id, std::function* on_valid, std::function* on_null) { return arrow::Status::NotImplemented("ActionBase Submit is abstract."); } arrow::Status ActionBase::Submit( - std::vector> in, + const std::vector>& in, std::function* on_valid, std::function* on_null) { return arrow::Status::NotImplemented("ActionBase Submit is abstract."); @@ -143,7 +143,7 @@ class UniqueAction : public ActionBase { #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -302,7 +302,7 @@ class CountAction : public ActionBase { #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -465,7 +465,7 @@ class CountDistinctAction : public ActionBase { #endif } std::string getName() { return "CountDistinctAction"; } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -616,7 +616,7 @@ class CountLiteralAction : public ActionBase { #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -740,7 +740,7 @@ class MinAction> #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -1022,7 +1022,7 @@ class MinAction> #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -1179,7 +1179,7 @@ class MinAction> #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -1350,7 +1350,7 @@ class MaxAction> #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -1627,7 +1627,7 @@ class MaxAction> #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -1783,7 +1783,7 @@ class MaxAction> #endif } - arrow::Status Submit(ArrayList in_list, int max_group_id, + arrow::Status Submit(const ArrayList& in_list, int max_group_id, std::function* on_valid, std::function* on_null) override { // resize result data @@ -1956,7 +1956,7 @@ class SumAction* on_valid, std::function* on_null) override { // resize result data @@ -2117,7 +2117,7 @@ class SumAction* on_valid, std::function* on_null) override { // resize result data @@ -2284,7 +2284,7 @@ class SumActionPartial* on_valid, std::function* on_null) override { // resize result data @@ -2450,7 +2450,7 @@ class SumActionPartial* on_valid, std::function* on_null) override { // resize result data @@ -2621,7 +2621,7 @@ class AvgAction* on_valid, std::function* on_null) override { // resize result data @@ -2792,7 +2792,7 @@ class AvgAction* on_valid, std::function* on_null) override { // resize result data @@ -2979,7 +2979,7 @@ class SumCountAction* on_valid, std::function* on_null) override { // resize result data @@ -3168,7 +3168,7 @@ class SumCountAction* on_valid, std::function* on_null) override { // resize result data @@ -3353,7 +3353,7 @@ class SumCountMergeAction* on_valid, std::function* on_null) override { // resize result data @@ -3534,7 +3534,7 @@ class SumCountMergeAction* on_valid, std::function* on_null) override { // resize result data @@ -3704,7 +3704,7 @@ class AvgByCountAction* on_valid, std::function* on_null) override { // resize result data @@ -3884,7 +3884,7 @@ class AvgByCountAction* on_valid, std::function* on_null) override { // resize result data @@ -4079,7 +4079,7 @@ class StddevSampPartialAction* on_valid, std::function* on_null) override { // resize result data @@ -4313,7 +4313,7 @@ class StddevSampPartialAction* on_valid, std::function* on_null) override { // resize result data @@ -4540,7 +4540,7 @@ class StddevSampFinalAction* on_valid, std::function* on_null) override { // resize result data @@ -4747,7 +4747,7 @@ class StddevSampFinalAction* on_valid, std::function* on_null) override { // resize result data @@ -4986,7 +4986,7 @@ class FirstPartialAction* on_valid, std::function* on_null) override { // resize result data @@ -5225,7 +5225,7 @@ class FirstPartialAction* on_valid, std::function* on_null) override { // resize result data @@ -5461,7 +5461,7 @@ class FirstFinalAction* on_valid, std::function* on_null) override { // resize result data @@ -5721,7 +5721,7 @@ class FirstFinalAction* on_valid, std::function* on_null) override { // resize result data diff --git a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.h b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.h index 7e025f37f..05a379831 100644 --- a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.h +++ b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.h @@ -40,10 +40,10 @@ class ActionBase { public: virtual ~ActionBase() {} - virtual arrow::Status Submit(ArrayList in, int max_group_id, + virtual arrow::Status Submit(const ArrayList& in, int max_group_id, std::function* on_valid, std::function* on_null); - virtual arrow::Status Submit(std::vector> in, + virtual arrow::Status Submit(const std::vector>& in, std::function* on_valid, std::function* on_null); virtual arrow::Status Submit(const std::shared_ptr& in, From f162215492c267b92314ca2b58108b030ef9fa08 Mon Sep 17 00:00:00 2001 From: philo Date: Fri, 19 Aug 2022 10:46:13 +0800 Subject: [PATCH 3/3] Remove pieces of unnecessary code --- .../cpp/src/codegen/arrow_compute/ext/actions_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc index 722ce2afd..e60ed1316 100644 --- a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc +++ b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/actions_impl.cc @@ -2302,7 +2302,7 @@ class SumActionPartial 0 && in_->IsNull(row_id); + const bool is_null = in_->IsNull(row_id); if (!is_null) { cache_validity_[dest_group_id] = true; cache_[dest_group_id] += data_[row_id]; @@ -2467,7 +2467,7 @@ class SumActionPartial 0 && in_->IsNull(row_id); + const bool is_null = in_->IsNull(row_id); if (!is_null) { cache_validity_[dest_group_id] = true; cache_[dest_group_id] += in_->GetView(row_id);