[GLUTEN-8557][CH] Flatten nested `And`/`Or` for performance optimization by KevinyhZou · Pull Request #8558 · apache/gluten

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

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

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

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

      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 ?

It seems can not

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

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


    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

    {
        const auto & args = substrait_func.arguments();
-        if (args.size() != 2)
+        if (args.size() < 2)


get_json_object(get_json_object(d, '$.a'), '$.b') => optimize to get_json_object(d, '$.a', '$.b'), which may have more than 2 arguments.

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

github-actions · 2025-03-06T13:16:06Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-03-07T04:14:58Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-03-07T12:37:22Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-03-08T04:41:16Z

Run Gluten Clickhouse CI on x86

PHILO-HE

Can we also introduce a dedicated rule for this optimization? Maybe, a custom And/Or expression class should be defined when necessary?

PHILO-HE · 2025-03-19T07:03:07Z

...use/src/main/scala/org/apache/gluten/expression/CHCollapseNestedExpressionsTransformer.scala

+    case _ => Option.empty[String]
+  }
+
+  private def canBeOptimized(expr: Expression): Boolean = {


For no nested And/Or case, is it also viewed as an optimizable case?

PHILO-HE · 2025-03-19T07:05:21Z

...use/src/main/scala/org/apache/gluten/expression/CHCollapseNestedExpressionsTransformer.scala

+
+  def getExpressionName(expr: Expression): Option[String] = expr match {
+    case _: And => ExpressionMappings.expressionsMap.get(classOf[And])
+    case _: Or => ExpressionMappings.expressionsMap.get(classOf[Or])


This check seems redundant to the configuration based check.

PHILO-HE · 2025-03-19T07:11:30Z

...use/src/main/scala/org/apache/gluten/expression/CHCollapseNestedExpressionsTransformer.scala

+          case _ => exprCall.children.exists(c => canBeOptimized(c))
+        }
+      case Some(f) =>
+        GlutenConfig.get.getSupportedCollapsedExpressions.split(",").exists(c => c.equals(f))


For mixed And/Or case, is it viewed as optimizable case?

github-actions · 2025-03-19T09:37:40Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-03-19T15:00:04Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-03-20T01:50:12Z

Run Gluten Clickhouse CI on x86

KevinyhZou · 2025-03-20T06:07:25Z

Can we also introduce a dedicated rule for this optimization? Maybe, a custom And/Or expression class should be defined when necessary?

Now I have implement a dedicated rule for this, introduce the CHAnd, CHOr customized expression for optimization.
Mixed nested and/or would be optimized, I have add this case in the ut, non nested and/or would not. please help to review this . @PHILO-HE

github-actions · 2025-03-27T03:15:44Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-03-27T03:52:16Z

Run Gluten Clickhouse CI on x86

KevinyhZou · 2025-03-28T07:21:13Z

cc @PHILO-HE

PHILO-HE

Basically looks good!

For such nested function optimization, it's a bit different from get_json_object optimization. So I would like to recommend to use "flatten" instead of "collapse" in the code.

Collapse => Flatten
Collapsed => Flattened

PHILO-HE · 2025-04-01T08:02:52Z

backends-clickhouse/src/main/scala/org/apache/gluten/expression/CHCollapsedExpression.scala

+    }
+}
+
+case class CHAnd(dataType: DataType, children: Seq[Expression], name: String, nullable: Boolean)


Suggest to use the below name to reflect its usage:
FlattenedAnd

PHILO-HE · 2025-04-01T08:03:11Z

backends-clickhouse/src/main/scala/org/apache/gluten/expression/CHCollapsedExpression.scala

+case class CHAnd(dataType: DataType, children: Seq[Expression], name: String, nullable: Boolean)
+  extends CHCollapsedExpression(children, name) {}
+
+case class CHOr(dataType: DataType, children: Seq[Expression], name: String, nullable: Boolean)


FlattenedOr

github-actions · 2025-04-01T08:41:41Z

Run Gluten Clickhouse CI on x86

KevinyhZou · 2025-04-01T08:43:27Z

Basically looks good!

For such nested function optimization, it's a bit different from get_json_object optimization. So I would like to recommend to use "flatten" instead of "collapse" in the code.

Collapse => Flatten Collapsed => Flattened

done

PHILO-HE

Thanks!

github-actions · 2025-04-02T03:47:58Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-02T06:12:58Z

Run Gluten Clickhouse CI on x86

taiyang-li

LGTM

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

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

taiyang-li reviewed Feb 12, 2025

View reviewed changes

lgbo-ustc reviewed Feb 12, 2025

View reviewed changes

KevinyhZou force-pushed the opt_nested_funcs branch from f4738d0 to 2c6d6f6 Compare March 7, 2025 12:36

PHILO-HE reviewed Mar 19, 2025

View reviewed changes

KevinyhZou force-pushed the opt_nested_funcs branch from fb730ea to 578808b Compare March 19, 2025 14:59

KevinyhZou force-pushed the opt_nested_funcs branch from 578808b to 2dbad5d Compare March 20, 2025 01:49

KevinyhZou force-pushed the opt_nested_funcs branch from 8ac225b to 68ad6fd Compare March 27, 2025 03:51

PHILO-HE changed the title ~~[GLUTEN-8557][CH] Collapse nested function calls for And/Or for performance optimization~~ [GLUTEN-8557][CH] Flatten nested And/Or for performance optimization Apr 1, 2025

PHILO-HE approved these changes Apr 1, 2025

View reviewed changes

KevinyhZou force-pushed the opt_nested_funcs branch from 68ad6fd to 0685f0c Compare April 1, 2025 08:41

PHILO-HE reviewed Apr 1, 2025

View reviewed changes

PHILO-HE approved these changes Apr 1, 2025

View reviewed changes

optimize nested and/or function calls

6d31ab9

KevinyhZou force-pushed the opt_nested_funcs branch from 7cc2137 to 6d31ab9 Compare April 2, 2025 06:12

taiyang-li approved these changes Apr 2, 2025

View reviewed changes

lgbo-ustc approved these changes Apr 3, 2025

View reviewed changes

taiyang-li merged commit 0918db2 into apache:main Apr 3, 2025
52 checks passed


		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

Conversation

KevinyhZou commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

github-actions bot commented Jan 17, 2025

Uh oh!

github-actions bot commented Jan 17, 2025

Uh oh!

github-actions bot commented Jan 21, 2025

Uh oh!

github-actions bot commented Jan 21, 2025

Uh oh!

github-actions bot commented Feb 4, 2025

Uh oh!

github-actions bot commented Feb 4, 2025

Uh oh!

github-actions bot commented Feb 5, 2025

Uh oh!

github-actions bot commented Feb 10, 2025

Uh oh!

github-actions bot commented Feb 10, 2025

Uh oh!

github-actions bot commented Feb 11, 2025

Uh oh!

KevinyhZou commented Feb 11, 2025

性能测试

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taiyang-li Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgbo-ustc commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgbo-ustc commented Feb 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgbo-ustc Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 7, 2025

Uh oh!

github-actions bot commented Mar 7, 2025

Uh oh!

github-actions bot commented Mar 8, 2025

Uh oh!

PHILO-HE left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 19, 2025

Uh oh!

github-actions bot commented Mar 19, 2025

Uh oh!

github-actions bot commented Mar 20, 2025

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

KevinyhZou commented Mar 20, 2025 •

edited

Loading

PHILO-HE left a comment •

edited

Loading

PHILO-HE Apr 1, 2025 •

edited

Loading