Fix: Explicit handle null pattern and escape in like function. #6894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for meta-velox canceled.
|
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 4, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). This added check in ConstantVector and also make sure like function handles null explicitly. Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
d809692 to
312326e
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 4, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). This added check in ConstantVector and also make sure like function handles null explicitly. Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
312326e to
2f41f2e
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 4, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). This added check in ConstantVector and also make sure like function handles null explicitly. Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
2f41f2e to
71d9d6d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 4, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). This added check in ConstantVector and also make sure like function handles null explicitly. Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
71d9d6d to
f2db3ef
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 4, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). This added check in ConstantVector and also make sure like function handles null explicitly. Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
f2db3ef to
c883bf4
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
laithsakka
force-pushed
the
export-D49917874
branch
from
c883bf4 to
f96d098
Compare
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 5, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). This added check in ConstantVector and also make sure like function handles null explicitly. Differential Revision: D49917874
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
mbasmanova
reviewed
Oct 5, 2023
velox/functions/lib/Re2Functions.cpp
Outdated
| @@ -470,6 +470,18 @@ class OptimizedLikeWithMemcmp final : public VectorFunction { | |||
| vector_size_t reducedPatternLength_; | |||
| }; | |||
|
|
|||
| // This functions is constructed when the input is a constant null. | |||
| class AlwaysNull final : public VectorFunction { | |||
There was a problem hiding this comment.
There is AlwaysFailingVectorFunction in VectorFunction.h. If this one doesn't work, let's add a new one in the same place so it can be reused.
laithsakka
added a commit
to laithsakka/velox
that referenced
this pull request
Oct 5, 2023
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). When i added a check in ConstantVector valueAt that value is not null i got other faluires I will address them in seperate PR then add the check. Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
f96d098 to
a5792a9
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
mbasmanova
reviewed
Oct 5, 2023
velox/expression/VectorFunction.h
Outdated
| // This functions is used when we know a function will never be called because | ||
| // it is default null with a literal null input value. For example like(c0, | ||
| // null). | ||
| class AlwaysNullDefaultNull final : public VectorFunction { |
There was a problem hiding this comment.
This name is a bit confusing to me. Wondering if we could renaming to something easier to understand. Maybe, NeverCalled? CC: @bikramSingh91
There was a problem hiding this comment.
NeverCalledDefaultNull? @mbasmanova is that good name ?
…ookincubator#6894) Summary: When value at constant vector is null we shall not call valueAt(). When i added a check in ConstantVector valueAt that value is not null i got other faluires I will address them in seperate PR then add the check. Reviewed By: mbasmanova Differential Revision: D49917874
laithsakka
force-pushed
the
export-D49917874
branch
from
a5792a9 to
0950cda
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49917874 |
|
This pull request has been merged in 37d8984. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
ericyuliu
pushed a commit
to ericyuliu/velox
that referenced
this pull request
Oct 12, 2023
…ookincubator#6894) Summary: Pull Request resolved: facebookincubator#6894 When value at constant vector is null we shall not call valueAt(). When i added a check in ConstantVector valueAt that value is not null i got other faluires I will address them in seperate PR then add the check. Reviewed By: mbasmanova Differential Revision: D49917874 fbshipit-source-id: db210ee2a777f1c4a1378d5d049a58389b08e646
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
When value at constant vector is null we shall not call valueAt().
When i added a check in ConstantVector valueAt that value is not null i got other faluires
I will address them in seperate PR then add the check.
Differential Revision: D49917874