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 #51636

Merged
merged 1 commit into from
Jul 27, 2020

Conversation

clucle
Copy link
Contributor

@clucle clucle commented Jul 21, 2020

Previously, Like matching has "prefix" match(test%) and "suffix" match(%test).

This commit adds "contains" match (%test%) using bytest.Contains function.

Release note (sql change): Add support for "contains" match(%test%) using bytes.Contains function

@blathers-crl
Copy link

blathers-crl bot commented Jul 21, 2020

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Jul 21, 2020
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rohany rohany requested a review from yuzefovich July 21, 2020 16:32
Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Seems like there is a conflict with tpch_vec again.

Reviewed 6 of 6 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@blathers-crl
Copy link

blathers-crl bot commented Jul 21, 2020

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @clucle)


pkg/sql/logictest/testdata/logic_test/tpch_vec, line 731 at r2 (raw file):

                    │       └ *colexec.mergeJoinInnerOp
                    │         ├ *colexec.selContainsBytesBytesConstOp
                    │         │ └ *colfetcher.colBatchScan

This will fail - colBatchScan has just been exported., so we need to keep colfetcher.ColBatchScan.

Previously, Like matching has "prefix" match(test%) and "suffix" match(%test).

This commit adds "contains" match (%test%) using bytest.Contains function.

Release note (sql change): Add support for "contains" match(%test%) using bytes.Contains function
@clucle
Copy link
Contributor Author

clucle commented Jul 21, 2020


pkg/sql/logictest/testdata/logic_test/tpch_vec, line 731 at r2 (raw file):

Previously, yuzefovich wrote…

This will fail - colBatchScan has just been exported., so we need to keep colfetcher.ColBatchScan.

Done.

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! bors r=yuzefovich

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)

@yuzefovich
Copy link
Member

bors r=yuzefovich

@yuzefovich
Copy link
Member

We're having a lot of issues with bors for some reason.

bors r=yuzefovich

@clucle
Copy link
Contributor Author

clucle commented Jul 22, 2020

We're having a lot of issues with bors for some reason.

bors r=yuzefovich

@yuzefovich

Do I need to rebase upstream/master and push again?

@yuzefovich
Copy link
Member

No need, our merging bot has been mysteriously crashing.

bors r=yuzefovich

@craig
Copy link
Contributor

craig bot commented Jul 22, 2020

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Jul 22, 2020

Build failed (retrying...)

@yuzefovich
Copy link
Member

bors r=yuzefovich

@craig
Copy link
Contributor

craig bot commented Jul 27, 2020

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants