You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/13 19:46:05 UTC

[GitHub] [arrow] jvanstraten commented on pull request #13537: ARROW-16988: [C++] Introduce Substrait ToProto/FromProto conversion options

jvanstraten commented on PR #13537:
URL: https://github.com/apache/arrow/pull/13537#issuecomment-1183610872

   > First, Is there a need/ask for PRESERVE_STRUCTURE?
   
   I have to admit I'm having a hard time coming up with a use case for it right now. It just feels right to keep it in there. By analogy with compiler flags:
   
    - PEDANTIC is like `gcc -Werror -Wall -pedantic-errors`: "if there is even the slightest hint of something maybe being wrong, please scream bloody murder."
    - PRESERVE_STRUCTURE is like `gcc -g -O0`: "if I write something stupid, please keep it exactly equally stupid, because I'm trying to figure out why my stuff isn't working." This doesn't really apply here (yet) because Substrait and (presumably) Acero both lack a means to annotate nodes with debug information, but I could still see it be useful when a user is debugging a query that somehow passes through Arrow.
    - BEST_EFFORT is like `gcc -O3` (or perhaps more accurately just `gcc` because we're not involving a proper optimization engine): "I just want my code/plan to work, and preferably work fast. Don't even tell me if I did something stupid, because I'm not interested in that right now."
   
   > Second, should the ExtensionSet be made an argument of conversion options?
   
   Maybe. It's part of the reason why I stuffed the enum in a struct. However, it's worth noting that ExtensionSet is mutated by the conversion, so it's not strictly speaking just an option.
   
   > Alternatively, we could bite the bullet and make the Substrait conversion object oriented.
   
   @vibhatha and I discussed this offline for a bit and agree that this would be better. See ARROW-16987. That would be a longer-term thing though, so I submitted this to not have the logic for these things be blocked by a possible refactoring later.
   
   ---
   
   I'll address the suggested changes to comments and the likes when we reach consensus about the options themselves.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org