-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Allow parsing byte literals as FixedSizeBinary #15686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
What about coercing the RHS to FixedSizeBinary? So like where byte_column = x'deadbeef' Becomes where byte_column = arrow_cast(x'deadbeef', FixedSizeBinary) This would have the benefit of:
|
@alamb thank you for looking at this. Avoiding a config flag would be nice. But I'm skeptical of the proposed coercion. If we coerce
Evaluates today to |
Here is a sql reproducer of what happens today > create table t as values (arrow_cast(x'deadbeef', 'FixedSizeBinary(4)'));
0 row(s) fetched.
Elapsed 0.006 seconds.
> select column1 = x'deadbeef' from t;
Error during planning: Cannot infer common argument type for comparison operation FixedSizeBinary(4) = Binary
I wonder how much less performant this in in practice 🤔 In theory we can have custom kernels with custom code for different array sizes, but I think in practice it actually ends up always checking the length I really think we should avoid a new config option if possible |
Ok I hear you, I'll try to do this without touching the parser and its configs. A concern with an LHS coercion is efficient pushdown. I will try to pursue the following approach:
If the lengths match, the cast is moved to the RHS. |
Is your feature request related to a problem or challenge?
Say you have a column
byte_column
of typeFixedSizeBinary
. Doingwhere byte_column = x'deadbeef'
will fail, because the literal is parsed asBinary
.Describe the solution you'd like
Add a option to parse literals as
FixedSizeBinary
.Describe alternatives you've considered
Coercing the LHS to
Binary
is less performant. Unconditionally parsing literals asFixedSizeBytes
would be a breaking change.Additional context
I am working on this.
The text was updated successfully, but these errors were encountered: