milesgranger commented on code in PR #14749:
URL: https://github.com/apache/arrow/pull/14749#discussion_r1035916807
##########
cpp/src/arrow/compute/kernels/scalar_nested.cc:
##########
@@ -131,13 +124,26 @@ struct ListSlice {
list_type->id() == arrow::Type::FIXED_SIZE_LIST);
std::unique_ptr<ArrayBuilder> builder;
+ // should have been checked in resolver
+ // if stop not set, then cannot return fixed size list without input being
fixed size
+ // list b/c we cannot determine the max list element in type resolving.
+ DCHECK(opts.stop.has_value() ||
+ (!opts.stop.has_value() && (!return_fixed_size_list ||
+ list_type->id() ==
arrow::Type::FIXED_SIZE_LIST)));
+
// construct array values
if (return_fixed_size_list) {
- RETURN_NOT_OK(MakeBuilder(
- ctx->memory_pool(),
- fixed_size_list(value_type,
- static_cast<int32_t>(opts.stop.value() -
opts.start)),
- &builder));
+ const auto stop = [&]() {
+ if (opts.stop.has_value()) {
+ return static_cast<int32_t>(opts.stop.value());
+ } else {
+ DCHECK_EQ(list_type->id(), arrow::Type::FIXED_SIZE_LIST);
+ return reinterpret_cast<const
FixedSizeListType*>(list_type)->list_size();
+ }
+ }();
Review Comment:
...because one can declare `stop` as a `const` for the cost of one extra
line? haha.
It actually spawned from my trying to satisfy MinGW thnking I was illegally
casting a different ListType than `FixedSizeList` in the second arm.. However,
in retrospect I think that was because I was using the ternary operator
`condition ? expr : expr` which MinGW apparently would type check both
expressions.
Anyway, I can remove the lambda if that's preferred.
--
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]
