-
Notifications
You must be signed in to change notification settings - Fork 837
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
Use Datum
for string kernels
#4632
Conversation
Really cool to see this, and thank you for breaking this up. I'm away for the next week but will try to get some time to sit down with this, but otherwise will get back to you on my return. I'm keen to see if there is some way to use type-erasure, i.e. |
I did a quick and dirty experiment, and it would appear my hunch is broadly correct
And got
Equality is the cheapest of the string operations and yet the dyn dispatch is barely registering. I suspect if you tweaked it to have a signature like |
Sorry for taking so long to get back to you, I think I would like this to follow a similar pattern as #4701 and #4465. I appreciate this is a much more intrusive change, and is quite fiddly, and so no hard feelings if you would like to back out of this. I already have a partially implemented version of this |
I was just reading #4701 :) Yeah I think that it will take me a while to figure this out completely. It might be more productive that you go further with it. Anyway, it allowed me to learn more about the code base. Feel free to close this PR! |
Closing in favor of #4732, thank you for your work on this 👍 |
Which issue does this PR close?
Closes #4595.
What changes are included in this PR?
For the moment I only implemented the change for
LIKE
and I would like your feedbacks before implementing other kernels.Are there any user-facing changes?
There will be some deprecated functions once it's finished.