[GLUTEN-8557][CH]Optimize nested function calls for `And`/`Or`/`GetStructField`/`GetJsonObject` #8558

KevinyhZou · 2025-01-17T09:10:57Z

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

(Fixes: #8557)

How was this patch tested?

test by ut

github-actions · 2025-01-17T09:11:16Z

#8557

github-actions · 2025-01-17T09:11:29Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-01-21T10:07:07Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-01-21T11:06:58Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-02-04T08:32:27Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-02-04T08:34:25Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-02-05T11:19:20Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-02-10T01:47:46Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-02-10T01:48:58Z

Run Gluten Clickhouse CI on x86

… into opt_nested_funcs

github-actions · 2025-02-11T03:28:49Z

Run Gluten Clickhouse CI on x86

KevinyhZou · 2025-02-11T09:08:48Z

性能测试

性能测试：测试数据量6千万行，分别执行三次，查看端到端执行时间

get_json_object: select count(1) from test_tbl1 where get_json_object(get_json_object(d, '$.a'), '$.b') = 'c';
优化前：53.194s, 52.18s, 51.603s
优化后：24.894s, 25.042s, 24.724s

and: select count(1) from test_tbl34 where id = 1 and d != 'axx' and d != 'cxx' and d != 'zxx';
优化前： 3.918s，4.106s，3.835s
优化后： 3.123s, 3.303s, 3.288s

or： select count(1) from test_tbl34 where id = 1 or d != 'axx' or d != 'cxx' or d != 'zxx';
优化前：2.611s，2.765s， 2.484s
优化后：2.3s, 2.409s, 2.283s;

get_struct_field: select count(1) from test_tbl31 where d.e.d.y = 'y123';
优化前：80.819s， 80.517s，83.3s
优化后：80.885s, 80.108s, 81.121s;

taiyang-li · 2025-02-12T02:14:10Z

shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala

+      .internal()
+      .doc("Collapse nested functions as one for optimization.")
+      .stringConf
+      .createWithDefault("get_struct_field,get_json_object");


what about and, or

and, or has conflict with ReusedExchange optimize rule, it would make the rule not take effect at some suitation, about 3 ut failed in the ci: https://opencicd.kyligence.com/job/gluten/job/gluten-ci/14509/consoleText. so I disable them by default.

taiyang-li · 2025-02-12T02:16:47Z

gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala

@@ -97,7 +97,10 @@ object ExpressionConverter extends SQLConfHelper with Logging {
    if (udf.udfName.isEmpty) {
      throw new GlutenNotSupportException("UDF name is not found!")
    }
-    val substraitExprName = UDFMappings.scalaUDFMap.get(udf.udfName.get)
+    var substraitExprName = UDFMappings.scalaUDFMap.get(udf.udfName.get)


collapsedFunctionsMap和udf有什么关系？看起来逻辑上没必要耦合在一块，最好能解耦

taiyang-li · 2025-02-12T02:19:17Z

backends-clickhouse/src/main/scala/org/apache/gluten/extension/CollapseNestedExpressions.scala

+import org.apache.spark.sql.execution.SparkPlan
+import org.apache.spark.sql.types.{DataType, DataTypes}
+
+case class CollapseNestedExpressions(spark: SparkSession) extends Rule[SparkPlan] {


add necessary comment

taiyang-li · 2025-02-12T02:20:28Z

backends-clickhouse/src/main/scala/org/apache/gluten/extension/CollapseNestedExpressions.scala

+  }
+
+  private def canBeOptimized(plan: SparkPlan): Boolean = plan match {
+    case p: ProjectExecTransformer =>


can expression in generate operator be optimized ?

taiyang-li · 2025-02-12T02:32:56Z

cpp-ch/local-engine/Functions/SparkFunctionGetJsonObject.h

@@ -462,8 +463,55 @@ class GetJsonObjectImpl

    static size_t getNumberOfIndexArguments(const DB::ColumnsWithTypeAndName & arguments) { return arguments.size() - 1; }

+    bool insertResultToColumn(DB::IColumn & dest, typename JSONParser::Element & root, std::vector<std::shared_ptr<DB::GeneratorJSONPath<JSONParser>>> & generator_json_paths, size_t & json_path_pos) const


@lgbo-ustc pls review changes related to get_json_object

lgbo-ustc · 2025-02-12T06:03:46Z

cpp-ch/local-engine/Parser/scalar_function_parser/getJSONObject.cpp

@@ -59,9 +59,9 @@ class GetJSONObjectParser : public FunctionParser
        DB::ActionsDAG & actions_dag) const override
    {
        const auto & args = substrait_func.arguments();
-        if (args.size() != 2)
+        if (args.size() < 2)


lgbo-ustc · 2025-02-12T06:18:30Z

解释下get_json_object的优化思路，不是太明白为何在这里要改成多个路径参数。
已经有 #8305 优化的情况下，这个改动带来哪些变化？

lgbo-ustc · 2025-02-12T06:18:50Z

建议不同函数的优化拆分到不同的PR里面

lgbo-ustc · 2025-02-12T06:30:33Z

cpp-ch/local-engine/Functions/SparkFunctionGetJsonObject.h

+    mutable size_t total_normalized_rows = 0;
+
+    template<typename JSONParser, typename JSONStringSerializer>
+    void insertResultToColumn(


It seems to be complex, I guess there should be a simpler implement with less branches

Should explain which case it is for each branch

lgbo-ustc · 2025-02-12T06:46:28Z

cpp-ch/local-engine/Functions/SparkFunctionGetJsonObject.h

+                    const char * query_begin = reinterpret_cast<const char *>(sub_field.c_str());
+                    const char * query_end = sub_field.c_str() + sub_field.size();


why not used the normalized one?

KevinyhZou marked this pull request as draft January 17, 2025 09:11

github-actions bot added CORE works for Gluten Core CLICKHOUSE labels Jan 17, 2025

KevinyhZou changed the title ~~[GLUTEN-8557][CH]Optimize nested function calls~~ [GLUTEN-8557][CH]Optimize nested function calls for And, GetStructField Jan 17, 2025

KevinyhZou changed the title ~~[GLUTEN-8557][CH]Optimize nested function calls for And, GetStructField~~ [GLUTEN-8557][CH]Optimize nested function calls for And/GetStructField Jan 17, 2025

KevinyhZou changed the title ~~[GLUTEN-8557][CH]Optimize nested function calls for And/GetStructField~~ [GLUTEN-8557][CH]Optimize nested function calls for And/Or/GetStructField/GetJsonObject Jan 21, 2025

KevinyhZou closed this Jan 21, 2025

KevinyhZou reopened this Jan 21, 2025

KevinyhZou marked this pull request as ready for review January 21, 2025 11:06

optimize nested function calls

2cc64f2

KevinyhZou force-pushed the opt_nested_funcs branch from b86d9e7 to 2cc64f2 Compare February 4, 2025 08:31

remove unused file

ba3befc

fix ci

16145f6

fix ci

5b3f83e

Merge branch 'main' into opt_nested_funcs

b76e3f9

KevinyhZou added 2 commits February 11, 2025 11:26

fix ci

2ec4cef

Merge branch 'opt_nested_funcs' of https://github.com/KevinyhZou/gluten…

8a95008

… into opt_nested_funcs

taiyang-li reviewed Feb 12, 2025

View reviewed changes

lgbo-ustc reviewed Feb 12, 2025

View reviewed changes

lgbo-ustc requested changes Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GLUTEN-8557][CH]Optimize nested function calls for `And`/`Or`/`GetStructField`/`GetJsonObject` #8558

[GLUTEN-8557][CH]Optimize nested function calls for `And`/`Or`/`GetStructField`/`GetJsonObject` #8558

KevinyhZou commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025

github-actions bot commented Jan 17, 2025

github-actions bot commented Jan 21, 2025

github-actions bot commented Jan 21, 2025

github-actions bot commented Feb 4, 2025

github-actions bot commented Feb 4, 2025

github-actions bot commented Feb 5, 2025

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 11, 2025

KevinyhZou commented Feb 11, 2025

taiyang-li Feb 12, 2025

KevinyhZou Feb 12, 2025

taiyang-li Feb 12, 2025

taiyang-li Feb 12, 2025

taiyang-li Feb 12, 2025

taiyang-li Feb 12, 2025 •

edited

Loading

lgbo-ustc Feb 12, 2025

lgbo-ustc commented Feb 12, 2025 •

edited

Loading

lgbo-ustc commented Feb 12, 2025

lgbo-ustc Feb 12, 2025

lgbo-ustc Feb 12, 2025 •

edited

Loading

lgbo-ustc Feb 12, 2025

		@@ -462,8 +463,55 @@ class GetJsonObjectImpl

		static size_t getNumberOfIndexArguments(const DB::ColumnsWithTypeAndName & arguments) { return arguments.size() - 1; }

		bool insertResultToColumn(DB::IColumn & dest, typename JSONParser::Element & root, std::vector<std::shared_ptr<DB::GeneratorJSONPath<JSONParser>>> & generator_json_paths, size_t & json_path_pos) const

		const char * query_begin = reinterpret_cast<const char *>(sub_field.c_str());
		const char * query_end = sub_field.c_str() + sub_field.size();

[GLUTEN-8557][CH]Optimize nested function calls for And/Or/GetStructField/GetJsonObject #8558

Are you sure you want to change the base?

[GLUTEN-8557][CH]Optimize nested function calls for And/Or/GetStructField/GetJsonObject #8558

Conversation

KevinyhZou commented Jan 17, 2025 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

github-actions bot commented Jan 17, 2025

github-actions bot commented Jan 17, 2025

github-actions bot commented Jan 21, 2025

github-actions bot commented Jan 21, 2025

github-actions bot commented Feb 4, 2025

github-actions bot commented Feb 4, 2025

github-actions bot commented Feb 5, 2025

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 11, 2025

KevinyhZou commented Feb 11, 2025

性能测试

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taiyang-li Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgbo-ustc commented Feb 12, 2025 • edited Loading

lgbo-ustc commented Feb 12, 2025

Choose a reason for hiding this comment

lgbo-ustc Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[GLUTEN-8557][CH]Optimize nested function calls for `And`/`Or`/`GetStructField`/`GetJsonObject` #8558

[GLUTEN-8557][CH]Optimize nested function calls for `And`/`Or`/`GetStructField`/`GetJsonObject` #8558

KevinyhZou commented Jan 17, 2025 •

edited

Loading

taiyang-li Feb 12, 2025 •

edited

Loading

lgbo-ustc commented Feb 12, 2025 •

edited

Loading

lgbo-ustc Feb 12, 2025 •

edited

Loading