sohailshk commented on code in PR #27352: URL: https://github.com/apache/datafusion/pull/27352#discussion_r4552147458
########## datafusion/functions/src/string/split_part.rs: ########## Review Comment: Both, depending on the path, and that difference is the bug. Sorry also for the description: it was the one from #27289 by mistake; it is now the split_part one. - Array path (`split_part_impl`, position from a column): the position is checked per row, and a row with any NULL argument is skipped first. So an all-NULL string column returns NULL, and a mixed batch errors on the first non-NULL row with position 0. - Fast path (`split_part_scalar`, array string with literal delimiter and position): the literal position was checked once per batch, before any row was read, so it errored even when every string was NULL. The change makes the fast path skip that check when no row has a string (`null_count() == len()`, which also covers the empty array the old guard handled), so both paths give the per-row result: an error exactly when some row has a non-NULL string, delimiter and position with position 0. A mixed batch still errors because its non-NULL row would error row by row too. With the literal position that means "the batch has a non-NULL string"; since that is a per-row condition, the outcome does not depend on how rows are split into batches. The new `expr.slt` cases cover the all-NULL column (literal position, position column, all literals) and the mixed batch that still errors. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
