Enable enable_dereference flag for spark_expression_fuzzer_test

[rui-mo]

Function row_constructor was only registered in Presto functions.
In RowConstructorCallToSpecialForm::constructSpecialForm, no registration
was found from function map due to the lack of registration in Spark functions.
Error occurs when accessing an invalid iterator. To fix this issue, this PR
registers row_constructor in Spark functions.
With this fix, enable_dereference flag can be enabled for
spark_expression_fuzzer_test.
#5967

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	20cbf35
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/65711a5279eaed00088410f6

[rui-mo]

@mbasmanova @kgpai Could you help review? Thank you.

[mbasmanova]

@rui-mo I wonder whether it would be cleaner to register kRowConstructor function in Spark.

[mbasmanova]

BTW, does Spark has something similar to kRowConstructor? Do semantics match?

[mbasmanova]

If Spark doesn't have row_constructor, then, perhaps, we should change the Fuzzer and not expect 'row_constructor' to be present. It should be possible to test dereference expression without row_constructor function. CC: @kagamiori

  if (options_.enableDereference) {
    addToTypeToExpressionListByTicketTimes("row", "row_constructor");
    addToTypeToExpressionListByTicketTimes(kTypeParameterName, "dereference");
  }

[mbasmanova]

What happens if you remove addToTypeToExpressionListByTicketTimes("row", "row_constructor"); from

  if (options_.enableDereference) {
    addToTypeToExpressionListByTicketTimes("row", "row_constructor");
    addToTypeToExpressionListByTicketTimes(kTypeParameterName, "dereference");
  }

?

[mbasmanova]

CC: @kagamiori

[rui-mo]

BTW, does Spark has something similar to kRowConstructor? Do semantics match?

@mbasmanova Spark does have similar function, and their semantics match. And we are using row_constructor function in Gluten for its computing.

https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala#L418-L434

I wonder whether it would be cleaner to register kRowConstructor function in Spark.

It makes sense to register it in Spark function from my view.

[mbasmanova]

@rui-mo Thank you for clarifying. Should we than chance this PR to register row_constructor for Spark?

[rui-mo]

@mbasmanova Thank you for the suggestion. I will make changes accordingly.

[kagamiori]

LGTM. Thank you for enabling row_constructor and enable_dereference!

[facebook-github-bot]

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[mbasmanova]

remove fmt::format

[rui-mo]

@mbasmanova @kagamiori Thanks for your review. Updated.

[facebook-github-bot]

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@kagamiori merged this pull request in 85df999.

[conbench-facebook]

Conbench analyzed the 1 benchmark run on commit 85df9992.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.