westonpace commented on PR #13537: URL: https://github.com/apache/arrow/pull/13537#issuecomment-1183680809
> 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. This is probably a workable explanation. My primary concern is that I want a user to easily know which option to pick. Can we update the description with something like "This may be useful if you are debugging plan execution and want to avoid optimizations that may restructure the plan." > 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. We have some precedence for this and signal it by passing the argument via pointer. For example, `SinkNodeOptions::generator`. However, in those cases we are usually pointing to "infrastructure". I understand your reasoning. I am not terribly fond of "options" objects that become "context" objects. > @vibhatha and I discussed this offline for a bit and agree that this would be better. See [ARROW-16987](https://issues.apache.org/jira/browse/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. If it's going to be refactored later then I'm fine with this for now. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
