Replace coalesce function with a special form #1430
Closed
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
facebook-github-bot
added
the
CLA Signed
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
438daab
to
3ac8654
Compare
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
3ac8654
to
c95406f
Compare
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
c95406f
to
f1e3c0c
Compare
|
@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
added a commit
to mbasmanova/velox-1
that referenced
this pull request
Apr 15, 2022
Summary: Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. Pull Request resolved: facebookincubator#1430 Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 1b430c3e863a8aaf14ed9cfd96a6118d6c11bb06
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
f1e3c0c
to
daf7624
Compare
|
This pull request was exported from Phabricator. Differential Revision: D35674128 |
|
This pull request was exported from Phabricator. Differential Revision: D35674128 |
mbasmanova
added a commit
to mbasmanova/velox-1
that referenced
this pull request
Apr 25, 2022
Summary: Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. Pull Request resolved: facebookincubator#1430 Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 85fec3a9569d56a1917824897162f61028b8ea82
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
daf7624
to
dfc4165
Compare
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 25, 2022
Summary: Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 07650ac8a519e566a96abdc27871c4f131478619
mbasmanova
added a commit
to mbasmanova/velox-1
that referenced
this pull request
Apr 25, 2022
Summary: X-link: pytorch/torcharrow#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. Pull Request resolved: facebookincubator#1430 Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 7cf3fac8650f6227beb4f6721935fa73de428397
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
dfc4165
to
9ac37cc
Compare
|
This pull request was exported from Phabricator. Differential Revision: D35674128 |
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 25, 2022
Summary: Pull Request resolved: pytorch#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: d1fe74c00ac355661ea90bf0d7837b7418ebb7e1
Summary: X-link: pytorch/torcharrow#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. Pull Request resolved: facebookincubator#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: bb9b73fb1c4160aee7111bf4dc08dcd3d68ebe57
|
This pull request was exported from Phabricator. Differential Revision: D35674128 |
mbasmanova
force-pushed
the
coalesce-special-form
branch
from
9ac37cc
to
67ea7e6
Compare
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 26, 2022
Summary: Pull Request resolved: pytorch#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 6ec63b4f5ecf99247232fb0f006209461a2b7662
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 26, 2022
Summary: Pull Request resolved: pytorch#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 83b6c13e9ff8a89aee988bbd705dd5e1831c4799
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 26, 2022
Summary: Pull Request resolved: pytorch#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 4b8d6599e74238b4a20a58c41c72de567b8a4da5
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 26, 2022
Summary: Pull Request resolved: pytorch#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 6bba174f60a7b1334dbcf8b89a7f245393cf1342
mbasmanova
added a commit
to mbasmanova/torcharrow
that referenced
this pull request
Apr 26, 2022
Summary: Pull Request resolved: pytorch#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: e7583dcabfa419ba5afcb425e6c58c5d64c9284b
facebook-github-bot
pushed a commit
to pytorch/torcharrow
that referenced
this pull request
Apr 27, 2022
Summary: Pull Request resolved: #301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 2d16cb3404843322da67b699e7163d53757225bd
| @@ -202,7 +202,7 @@ class FlatVector final : public SimpleVector<T> { | |||
| T* mutableRawValues() { | |||
| if (!values_ || !values_->unique()) { | |||
| BufferPtr newValues = | |||
| AlignedBuffer::allocate<T>(BaseVector::length_, values_->pool()); | |||
| AlignedBuffer::allocate<T>(BaseVector::length_, BaseVector::pool()); | |||
There was a problem hiding this comment.
QQ : Why is this needed for the current change ?
There was a problem hiding this comment.
This is needed to avoid a crash when values_ is a nullptr.
Arty-Maly
pushed a commit
to Arty-Maly/velox
that referenced
this pull request
May 13, 2022
Summary: X-link: pytorch/torcharrow#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. Pull Request resolved: facebookincubator#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 2d16cb3404843322da67b699e7163d53757225bd
facebook-github-bot
pushed a commit
to pytorch/torcharrow
that referenced
this pull request
Jun 29, 2022
Summary: X-link: #301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. X-link: facebookincubator/velox#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 2d16cb3404843322da67b699e7163d53757225bd
shiyu-bytedance
pushed a commit
to shiyu-bytedance/velox-1
that referenced
this pull request
Aug 18, 2022
Summary: X-link: pytorch/torcharrow#301 Coalesce needs to be a special form expression and cannot be a function because it is not supposed to evaluate remaining inputs once there are no nulls. For example, if column a has no nulls, the following expression should not try to evaluate `b / 0` and should not fail. ``` coalesce(a, b / 0) ``` This change introduces CoalesceExpr special form and deletes coalesce function. Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues. Pull Request resolved: facebookincubator#1430 Reviewed By: laithsakka Differential Revision: D35674128 Pulled By: mbasmanova fbshipit-source-id: 2d16cb3404843322da67b699e7163d53757225bd
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.
Coalesce needs to be a special form expression and cannot be a function because
it is not supposed to evaluate remaining inputs once there are no nulls. For
example, if column a has no nulls, the following expression should not try to
evaluate
b / 0and should not fail.This change introduces CoalesceExpr special form and deletes coalesce function.
Also, fix a crash due to null pointer dereference in FlatVector::mutableRawValues.