Add FixedSizeBinary support to binary_op_dyn_scalar #6891
Conversation
github-actions
bot
added
sql
There was a problem hiding this comment.
Thanks @maxburke -- the code change looks good to me, but I think there are unrelated changes in this PR for some reason. Once that is resolved I think this PR is good to go.
I also think this change should be tested so that we don't break the feature in the future. I tried to add some basic coverage here: #6895 so I think this PR can be merged once the extraneous version stuff is taken care of
Cargo.toml
Outdated
| @@ -35,7 +35,7 @@ members = [ | |||
| ] | |||
|
|
|||
| [workspace.package] | |||
| version = "26.0.0" | |||
| version = "27.0.0" | |||
There was a problem hiding this comment.
It is strange the diff shows these changes as they seem to have been made on master 🤔
| @@ -1005,6 +1005,7 @@ macro_rules! binary_array_op_dyn_scalar { | |||
| ScalarValue::LargeUtf8(v) => compute_utf8_op_dyn_scalar!($LEFT, v, $OP, $OP_TYPE), | |||
| ScalarValue::Binary(v) => compute_binary_op_dyn_scalar!($LEFT, v, $OP, $OP_TYPE), | |||
| ScalarValue::LargeBinary(v) => compute_binary_op_dyn_scalar!($LEFT, v, $OP, $OP_TYPE), | |||
| ScalarValue::FixedSizeBinary(_, v) => compute_binary_op_dyn_scalar!($LEFT, v, $OP, $OP_TYPE), | |||
There was a problem hiding this comment.
I think this is the only actual code change
I wrote some tests for it here: #6895
github-actions
bot
removed
sql
Weird! Alright, I rebased the change on main and the only relevant change is now included. |
|
FYI this PR may not be enough to compared fixed size binary -- see #6895 for details |
Closes #6890