bkietz commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995788662
##########
cpp/src/arrow/engine/substrait/serde.h:
##########
@@ -261,10 +261,17 @@ Status CheckMessagesEquivalent(std::string_view
message_name, const Buffer& l_bu
///
/// \param[in] type_name the name of the Substrait message type to convert
/// \param[in] json the JSON string to convert
+/// \param[in] ignore_unknown_fields if true then unknown fields will be
ignored and
+/// will not cause an error
+///
+/// This should generally be true to allow consumption of plans
from newer
+/// producers but setting to false can be useful if you are testing
+/// conformance to a specific Substrait version
/// \return a buffer filled with the binary protobuf serialization of message
ARROW_ENGINE_EXPORT
Result<std::shared_ptr<Buffer>> SubstraitFromJSON(std::string_view type_name,
- std::string_view json);
+ std::string_view json,
+ bool ignore_unknown_fields =
false);
Review Comment:
I think the default should probably be `true`, since this is the current
behavior expected by the bindings which use `SubstraitFromJSON` without the
explicit argument. Otherwise if you feel like being fancy we could get the
default from an environment variable...
```suggestion
bool ignore_unknown_fields
= true);
```
--
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]
