search

apache / datafusion-comet

Apache DataFusion Comet Spark Accelerator
https://datafusion.apache.org/comet
Apache License 2.0
823 stars 163 forks source link

feat: Implement to_string for subtype #980

Closed adi-kmt closed 1 week ago

adi-kmt commented 1 month ago

Which issue does this PR close?

Closes #814

Rationale for this change

What changes are included in this PR?

Adding the To string cast

How are these changes tested?

Rust tests are written and tested.

adi-kmt commented 1 month ago

Hey @andygrove @parthchandra @viirya let me know if this is good enough or something has to be improved

andygrove commented 1 month ago

Apologies @adi-kmt I had not seen this PR. I will start reviewing today.

andygrove commented 1 month ago

There is a build error:

Error:  /__w/datafusion-comet/datafusion-comet/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala:1333: not found: value StructsToString
andygrove commented 3 weeks ago

Hi @adi-kmt. I just spent some time looking through this PR and I think there is some confusion that I can help with. The issue https://github.com/apache/datafusion-comet/issues/814 is for implementing CAST from struct to string. In SQL, this would look like CAST(struct_value AS string) rather than to_string (which does not seem to exist in Spark?). There should be no need to add additional protobuf types since we already have CAST supported there.

CometCastSuite may be a good place to start with adding a test. Let me know if you have questions.

andygrove commented 1 week ago

This is now implemented in https://github.com/apache/datafusion-comet/pull/1066