-
Notifications
You must be signed in to change notification settings - Fork 1.8k
enhancement(enriching): add optional wildcard search parameter #23074
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
Conversation
Hi @buraizu, thank you for your review and approval. By any chance, are you able to contact one of the vector maintainers to review this PR as well? |
Hi @pront , sorry to bug you directly. Would you mind reviewing this PR when you have some time? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality looks good. Docs needs some improvement. I also left some nit comments (but those are optional).
website/cue/reference/remap/functions/find_enrichment_table_records.cue
Outdated
Show resolved
Hide resolved
Hi @pront thank you for the review. I appreciate your diligence, but would it be okay to merge this without the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
Sorry for all the noise. I didn't realize that the website docs get parsed into actual tests. This time, I confirmed that |
No problem, thank you for the fixes! |
Summary
This PR resolves #22920 by enhancing the enrichment functions with support for optional matching against a wildcard as a fallback. It's implementation is a fallback to finding a match against the wildcard in the
indexed
function to pass the wildcard-matched rows to the follow-up sequential scan which was also updated to include a fallback to matching against the wildcard inrow_equals
. In the latter, I've refactored the comparison logic tocompare_values
as I believe its more readable (but I'm happy to be wrong about this). I also tried to update the documentation but I'm happy to correct this as well.As a side note, I did consider implementing some
delimiter
parameter which would allow the user to specifydelimiter="|"
and pass multiple match values per column in the condition field directly but the implementation would be more complicated than simply specifying a wildcard for all fields. Full regex support would be even better but its implementation would be even more complicated.Change Type
Is this a breaking change?
How did you test this PR?
Does this PR include user facing changes?