substrait-io
docs: clarify advanced extensions
The Advanced Extensions section of the docs seemed somewhat mismatched from what was in the protobufs, so I tried to clarify it, based on what was in the protobuf definitions.
Thoughts welcome! Also could use some sanity-checking on the details!
Naming
I also included custom relations (ExtensionLeafRel, …) and custom reads and writes (ExtensionTable, …) all under the heading "Advanced Extensions", even though there is an AdvancedExtension message that doesn't cover those. The name "Advanced Extension" seems a bit ambiguous here - does it cover all of the above, or only the enhancements and optimizations in the AdvancedExtension message? - but it seems to be what we have, so I went with it. Thoughts?
Guidance
This is a bit low on guidance on when to use which - e.g. ExtensionLeafRel, ExtensionTable, or ReadRel.advanced_extension.enhancement could potentially all be used for an unusual kind of read. I'm not sure what the guidance here should be, or if there should be any, so I left it out; I'm not sure any guidance here would be that helpful.
@yongchul - thanks for the review! Good comments - I've updated to address them. Can you give them a look?
Thanks!
Thanks again! Incorporated. I also realized I had the !!! note sections incorrect (they needed indenting), so I fixed that, and ran mkdocs locally to verify it worked - and it did!
^ Thanks, @yongchul for the merge! I'm wondering - what do we need to do next to advance this? Do we need additional reviewers so we can get it approved and merged?
^ Thanks, @yongchul for the merge! I'm wondering - what do we need to do next to advance this? Do we need additional reviewers so we can get it approved and merged?
Thank you for your patience! We will need another SMC review I believe. I'll bring this up in the sync up.
@yongchul @wackywendell As I understand it, the governance rules dictate that only one SMC approval is necessary to get documentation changes in.
Since @yongchul is an SMC, do we have to wait for any other approvals? Or can we just merge it?
@yongchul @wackywendell As I understand it, the governance rules dictate that only one SMC approval is necessary to get documentation changes in.
Since @yongchul is an SMC, do we have to wait for any other approvals? Or can we just merge it?
I'm going to merge after re-reading the changes one last time. :smile
@wackywendell thank you for the changes and sorry about taking it so long! Please resolve the two minor changes and it looks good to me to go!
Thanks, @yongchul! I updated, let me know how it looks to you - especially the sorting example, clustering was unclear as you noted!
The one issue I see with the extensions updates here is that they act like the only way to express something is using protobuf. Our goal was to spec first, with proto being an implementation. That being said, the reality is a bit different than that.
