-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Cleanup regex_expressions.rs to remove _regexp_match function #9107
Conversation
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 @Omega359 -- the code looks great. I am running some benchmarks just to be sure
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.
@viirya fixed the problem in #8631
I ran some of the queries from that PR:
❯ SELECT COUNT(*) FROM 'index' WHERE
ARRAY_LENGTH(
REGEXP_MATCH(path, '\\.(asm|c|cc|cpp|cxx|h|hpp|rs|[Ff][0-9]{0,2}(?:or)?|go)$')
) > 0;
on 7.2 GB of parquet data
$ du -s -h index
7.2G index
Main (without this PR):
1 row in set. Query took 4.820 seconds.
This PR:
1 row in set. Query took 4.760 seconds.
So basically the same.
Thank you @Omega359 -- very nice cleanup 🧹
Removed extraneous line as per code review suggestion Co-authored-by: Liang-Chi Hsieh <[email protected]>
Removed extraneous lines as per code review Co-authored-by: Liang-Chi Hsieh <[email protected]>
Which issue does this PR close?
Closes #9106
Rationale for this change
Code cleanup.
What changes are included in this PR?
Code removal, minor code cleanup, toml file updates.
Are these changes tested?
Yes
Are there any user-facing changes?
No.