alamb commented on a change in pull request #9944:
URL: https://github.com/apache/arrow/pull/9944#discussion_r610150874
##########
File path: rust/datafusion/src/physical_plan/crypto_expressions.rs
##########
@@ -144,7 +144,7 @@ fn md5_array<T: StringOffsetSizeTrait>(
}
/// crypto function that accepts Utf8 or LargeUtf8 and returns a
[`ColumnarValue`]
-pub fn md5(args: &[ColumnarValue]) -> Result<ColumnarValue> {
+pub fn md5(args: &[ColumnarValue], _: &Schema) -> Result<ColumnarValue> {
Review comment:
Yeah, it is unfortunate that we had to plumb the `schema` argument all
the way through.
Another pattern I have seen (though it has its own downsides) is to use some
sort of `thread_local` storage to pass stuff like this (from the `RecordBatch`s
schema into the expression evaluation code).
So like before evaluating a physical_expr we would set some sort of
thread_local that had a pointer back to the `Schema` that `input_file_name`
could consult. This would avoid a bunch of plumbing through a mostly unused
argument all over the place.
What do you think of this approach @jorgecarleitao and @Dandandan ?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
