jvanstraten commented on PR #12916: URL: https://github.com/apache/arrow/pull/12916#issuecomment-1105027656
I put it there (and it should also be in the C++ file) such that if someone modifies one, it's relatively obvious that they should also modify the other. I'd say that the mere existence of a JIRA somewhere is rather unlikely to accomplish that goal, but that's just my opinion; if it goes against Arrow project conventions, then remove it. That said... in my mind this isn't (going to be) an implementation-agnostic file at all, it describes exactly what the Arrow C++ consumer of Substrait understands. Right now it only specifies the types, but Substrait currently doesn't specify a way for YAML files to refer to each other, so unless support for that is added, all the functions would need to go in the same YAML file, or they wouldn't be able to use those types. And supporting that is hardly trivial -- how would you determine if two generalized URIs refer to the same file? I suppose you could also argue that the YAML file defines what a generalized Arrow Substrait consumer *should* support. In that case, though, ARROW-15535 should be closed and the comments at https://github.com/apache/arrow/blob/1dccb56a8328abde60238a890f720d7cc642cc94/cpp/src/arrow/engine/substrait/extension_set.cc#L223 and https://github.com/apache/arrow/blob/1dccb56a8328abde60238a890f720d7cc642cc94/cpp/src/arrow/engine/substrait/extension_set.cc#L248 should be updated accordingly, because it makes no sense to generate something that's implementation-agnostic from a particular implementation. It'd be a lot of manual, highly error-prone work to treat the file that way, though. -- 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]
