Skip to content
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

colexec: add optimized support of "contains" LIKE match #51584

Closed
yuzefovich opened this issue Jul 19, 2020 · 7 comments
Closed

colexec: add optimized support of "contains" LIKE match #51584

yuzefovich opened this issue Jul 19, 2020 · 7 comments
Assignees
Labels
A-sql-vec SQL vectorized engine C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) good first issue

Comments

@yuzefovich
Copy link
Member

We currently have optimized support of LIKE matching for "prefix" match (test%) and "suffix" match (%test). We could also easily add support for "contains" match (%test%) using bytes.Contains function.

This will require the following:

  1. updating execgen/like_ops_gen.go to introduce the new "overload" for LIKE operator to be generated (you'll need to run make execgen to regenerate the code)
  2. updating colexec/like_ops.go to support new "contains" likeOpType
  3. updating GetLikeOperator and GetLikeProjectionOperator methods accordingly
  4. adding some tests in logic_test/vectorize.
@yuzefovich yuzefovich added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) good first issue A-sql-vec SQL vectorized engine labels Jul 19, 2020
@yuzefovich yuzefovich added this to Triage in BACKLOG, NO NEW ISSUES: SQL Execution via automation Jul 19, 2020
@clucle
Copy link
Contributor

clucle commented Jul 19, 2020

Hi @yuzefovich I'd like to work this issue if no one is working on it.

@clucle
Copy link
Contributor

clucle commented Jul 19, 2020

Should I add BenchmarkLikeOps in execgen/like_ops_test.go for "contains"?

@yuzefovich
Copy link
Member Author

Hi @clucle, thanks for the interest! I'll assign the issue to you.

In terms of benchmarks, you should adjust BenchmarkLikeOps in colexec/like_ops_test.go to include the "contains" pattern. Also, it'd be nice to add a couple of unit test cases in TestLikeOperators.

@yuzefovich yuzefovich moved this from Triage to [VECTORIZED BACKLOG] Enhancements/Features/Investigations in BACKLOG, NO NEW ISSUES: SQL Execution Jul 19, 2020
@clucle
Copy link
Contributor

clucle commented Jul 20, 2020

{
pattern: "%e%",
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"def"}},
},
{
pattern: "%e%",
negate: true,
tups: tuples{{"abc"}, {"def"}, {"ghi"}},
expected: tuples{{"abc"}, {"ghi"}},
},

In my opinion, it seems that TestLikeOperators already contain the test, so I didn't reflect it.

Should I add a test case?

@clucle
Copy link
Contributor

clucle commented Jul 20, 2020

@yuzefovich

I don't know how to add a reviewer( #51598 ) because this is my first contribution to this project.

Help me plz 😄

@yuzefovich
Copy link
Member Author

In my opinion, it seems that TestLikeOperators already contain the test, so I didn't reflect it.

Oh yeah, I didn't see it. No need for extra test case then.

@yuzefovich
Copy link
Member Author

This issue has been addressed by #51636.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-vec SQL vectorized engine C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) good first issue
Projects
BACKLOG, NO NEW ISSUES: SQL Execution
[VECTORIZED BACKLOG] Enhancements/Fea...
Development

No branches or pull requests

2 participants