I'm curious as to why we return an error here. Why not set the limit to the lower of the 2 instead?

vec![1,2,3,4,5].iter().take(7) doesn't throw an error, and SELECT * FROM table_with_100_records LIMIT 200 will also only return the 100 records. What do you think about the suggestion?

My initial thought was that if someone calls this function with a num_rows_to_read larger than array.len(), they will probably want to know rather than silently choosing the minimum.

However given take and LIMIT behavior this is indeed probably not what is expected from this function. I will change it, good call.

I think if you limit the num_rows_to_read to the array length.
Have a look at https://issues.apache.org/jira/browse/ARROW-3954, which will make filter more efficient in future.

One last nit, we could return the array as ArrayRef immediately if the limit >= len. I sold have thought of it earlier, my apologies.

I'm happy with everything else

Maybe you can help me here: how can I wrap the reference to array in an Arc<Array> / ArrayRef? Since the reference to array can be freed at any time it would leave the Arc invalid, so I have to specify the lifetime somehow. Can I do that with Arc?

No, you're right, I missed that part. We can improve limit when we have zero-copy array slicing 👍🏾

It would be more efficient to initialize these vectors with Vec::with_capacity(v.len())

Sorry, I guess this should be Vec::with_capacity(v.len() + 1). Also you have an extra ;

Thanks!

After reviewing this again, I realize that this change is good for offsets but maybe not for values. We can only allocate values to the correct size if we first go and sum the lengths of all of the input strings, which would be expensive. Maybe we should just go back to vec![] or Vec::new() for this one?

Ah, yes. Also since v can contain empty strings, we do not even have a lower bound for an initial capacity, so I'll revert to Vec::new() for values.