apache / datafusion-comet

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

docs: add guide to adding a new expression #422

Closed tshauck closed 3 weeks ago

tshauck commented 1 month ago

Which issue does this PR close?

Part of #370 (haven't added anything on cast or aggregate, mainly scalar func)

Rationale for this change

Make it easier for people to contribute new expressions.

What changes are included in this PR?

A new markdown file in the contributor guide.

How are these changes tested?

~Not yet, I haven't tried building the docs. Will do before marking this PR ready.~

Built the docs and reviewed the HTML.

tshauck commented 1 month ago

Notes from comet community meeting:

Questions:

andygrove commented 1 month ago

@tshauck video link is https://drive.google.com/file/d/1POU4lFAZfYwZR8zV1X2eoLiAmc1GDtSP/view?usp=sharing

tshauck commented 1 month ago

I think this is ready for review. The first draft I wrote was pretty slanted towards scalar function expressions since that's what I did w/ unhex, but after the meeting on Wed, I made some updates so hopefully it's more general. I'm not 100% sure on the things I haven't touched yet, so certainly open to feedback on any of it.

Here's what it looks like included in the docs...

image
andygrove commented 4 weeks ago

This is looking great @tshauck. Could you fix the merge conflict?

tshauck commented 4 weeks ago

Thanks, @andygrove -- I think the conflict is resolved now.

tshauck commented 3 weeks ago

@andygrove Conflict should actually be fixed now, thanks @kazuyukitanimura