westonpace commented on code in PR #14402:
URL: https://github.com/apache/arrow/pull/14402#discussion_r995161345
##########
cpp/src/arrow/engine/substrait/expression_internal.cc:
##########
@@ -295,7 +295,8 @@ Result<compute::Expression> FromProto(const
substrait::Expression& expr,
Result<Datum> FromProto(const substrait::Expression::Literal& lit,
const ExtensionSet& ext_set,
const ConversionOptions& conversion_options) {
- if (lit.nullable()) {
Review Comment:
I don't think that comment says it must be set to false. It simply says the
field does not apply.
I'm not sure I support being pedantic here for no reason as we are not a
Substrait validator. In general, if an option doesn't make sense, but we can
safely ignore it, I think we should.
--
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]
