feat: Add support for CreateNamedStruct #620
Conversation
eejbyfeldt
force-pushed
the
support-create-named-struct
branch
from
b232326
to
5cf3264
Compare
| .setCreateNamedStruct(structBuilder) | ||
| .build()) | ||
| } else { | ||
| withInfo(expr, struct.valExprs: _*) |
There was a problem hiding this comment.
I think we need to specify an info string here otherwise the user will not see the reason for falling back to Spark (perhaps @parthchandra can confirm).
| withInfo(expr, struct.valExprs: _*) | |
| withInfo(expr, "unsupported arguments for CreateNamedStruct", struct.valExprs: _*) |
There was a problem hiding this comment.
Based on this comment here
it sounds like we would still get the errors from the children which should contain the root issue. So it not clear to me we would be able to provide any additional useful information here.
There was a problem hiding this comment.
I think we need to specify an info string here otherwise the user will not see the reason for falling back to Spark (perhaps @parthchandra can confirm).
The user will get the reason from the child nodes. We could add the additional message as @andygrove has suggested, but it is not strictly necessary.
There was a problem hiding this comment.
Resolved, by adding the message. I think there is some argument to made around this making it harder to figure out the root cause since there is more information in the info log.
eejbyfeldt
force-pushed
the
support-create-named-struct
branch
from
7d6ad4b
to
9463dde
Compare
spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
Outdated
Show resolved
Hide resolved
eejbyfeldt
force-pushed
the
support-create-named-struct
branch
from
9463dde
to
350e8d7
Compare
eejbyfeldt
force-pushed
the
support-create-named-struct
branch
from
350e8d7
to
9a71e40
Compare
eejbyfeldt
force-pushed
the
support-create-named-struct
branch
from
9a71e40
to
45d913c
Compare
|
Thanks @eejbyfeldt |
* Add support for CreateNamedStruct * Exclude HashJoins using struct keys as this is currently unsupported in datafusion * Add CreateNamedStruct to docs * Add message
Which issue does this PR close?
Closes #619.
Rationale for this change
Add support for one more expression.
What changes are included in this PR?
Adds a custom
PhysicalExprthat implements the behavior of the CreateNamedStruct in spark.How are these changes tested?
New unit tests in the CometExpressionSuite.