LiaCastaneda commented on code in PR #24162:
URL: https://github.com/apache/datafusion/pull/24162#discussion_r3756621582
##########
datafusion/expr/src/higher_order_function.rs:
##########
@@ -365,23 +403,43 @@ fn merge_captures_with_variables(
);
}
+ let push_param_arrays = |columns: &mut Vec<ArrayRef>| -> Result<()> {
+ for &i in used_param_indices {
+ columns.push(variables[i]()?);
+ }
+ Ok(())
+ };
+
let columns = match captures {
Some(captures) => {
let mut columns = captures.columns().to_vec();
-
- for arg in &variables[..params.len()] {
- columns.push(arg()?);
- }
-
+ push_param_arrays(&mut columns)?;
+ columns
+ }
+ None => {
+ let mut columns = Vec::with_capacity(used_param_indices.len());
+ push_param_arrays(&mut columns)?;
columns
}
- None => variables
- .iter()
- .take(params.len())
- .map(|arg| arg())
- .collect::<Result<_>>()?,
};
+ if columns.is_empty() {
+ // Constant lambda body with no captures and no used parameters. We
+ // still need a row count for the merged batch, so evaluate one
+ // variable just to derive it. This is essentially free in the common
+ // case (the variables already exist as closures over arrays the
+ // caller computed up front).
+ let row_count = match variables.first() {
+ Some(first) => first()?.len(),
+ None => 0,
+ };
+ return Ok(RecordBatch::try_new_with_options(
+ schema,
+ vec![],
+ &RecordBatchOptions::new().with_row_count(Some(row_count)),
+ )?);
+ }
+
Review Comment:
sort of introduced by our changes. Before, we pushed all params (used or
not) into `columns` so the variable was never empty. Now we just push the used
params only, so for lambdas that use 0 params (like `(k,v) -> constant`)
`columns` is empty, so we need to derive the row count manually (otherwise
`RecordBatch::try_new(schema, columns) `wont do it, like before)
--
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]
