Refine the code

oap-project · Oct 12, 2022 · 77300c2 · 77300c2
1 parent da83fb0
commit 77300c2
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 14 deletions.
diff --git a/native-sql-engine/cpp/src/codegen/arrow_compute/ext/window_kernel.cc b/native-sql-engine/cpp/src/codegen/arrow_compute/ext/window_kernel.cc
@@ -1023,7 +1023,7 @@ arrow::Status WindowSumKernel::Finish(ArrayList* out) {
 
   // The above is almost as same as Rank's except using sort (order by) input for sorter.
 
-#define PROCESS_SUPPORTED_COMMON_TYPES_LAG(PROC)                                                          \
+#define PROCESS_SUPPORTED_COMMON_TYPES_SUM(PROC)                                                          \
   PROC(arrow::UInt8Type, arrow::UInt8Array, arrow::Int64Type, arrow::Int64Builder, arrow::Int64Array)     \
   PROC(arrow::Int8Type, arrow::Int8Array, arrow::Int64Type, arrow::Int64Builder, arrow::Int64Array)       \
   PROC(arrow::UInt16Type, arrow::UInt16Array, arrow::Int64Type, arrow::Int64Builder, arrow::Int64Array)   \
@@ -1046,15 +1046,9 @@ arrow::Status WindowSumKernel::Finish(ArrayList* out) {
         values, group_ids, max_group_id, sorted_partitions, out,                           \
         get_nonstring_value<ARRAY_TYPE, CType>)));                                         \
   } break;
-    PROCESS_SUPPORTED_COMMON_TYPES_LAG(PROCESS)
+    PROCESS_SUPPORTED_COMMON_TYPES_SUM(PROCESS)
 #undef PROCESS
-#undef PROCESS_SUPPORTED_COMMON_TYPES_LAG
-    // case arrow::StringType::type_id: {
-    //   RETURN_NOT_OK((HandleSortedPartition<arrow::StringType, std::string,
-    //                                        arrow::StringBuilder, arrow::StringArray>(
-    //       values, group_ids, max_group_id, sorted_partitions, out,
-    //       get_string_value<arrow::StringArray, std::string>)));
-    // } break;
+#undef PROCESS_SUPPORTED_COMMON_TYPES_SUM
     default: {
       return arrow::Status::Invalid("window function: unsupported input type: " +
                                     value_type->name());
@@ -1095,6 +1089,7 @@ arrow::Status WindowSumKernel::HandleSortedPartition(
                 values.at(index->array_id).at(column_id));
         // If the first value in one partition (ordered) is null, the result is null.
         // If there is valid value before null, the result for null is as same as the above.
+        // So for same value in ordered col, the sum result may be different from vanilla's. 
         if (typed_array->null_count() > 0 && typed_array->IsNull(index->id)) {
           if (!is_valid_value_found) {
             validity[index->array_id][index->id] = false;

diff --git a/native-sql-engine/cpp/src/tests/arrow_compute_test_window.cc b/native-sql-engine/cpp/src/tests/arrow_compute_test_window.cc
@@ -471,17 +471,14 @@ TEST(TestArrowComputeWindow, SumOrderedTest) {
   std::vector<std::string> input_data = {"[1, 2, 1]", "[39, 37, 38]"};
   MakeInputBatch(input_data, sch, &input_batch);
 
-  std::shared_ptr<Field> res = field("window_res", arrow::int32());
+  std::shared_ptr<Field> res = field("window_res", arrow::int64());
 
   auto f_window = TreeExprBuilder::MakeExpression(
       TreeExprBuilder::MakeFunction(
           "window",
           {TreeExprBuilder::MakeFunction(
                "sum_desc",
-               {TreeExprBuilder::MakeField(field("col_dec", arrow::int32())),
-                // offset is 1, default value is null.
-                TreeExprBuilder::MakeLiteral((int)1),
-                TreeExprBuilder::MakeNull(arrow::int32())},
+               {TreeExprBuilder::MakeField(field("col_dec", arrow::int32()))},
                null()),
            TreeExprBuilder::MakeFunction(
                "partitionSpec",