You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/11 18:36:40 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #2875: Alamb/fix cast dicts
alamb opened a new pull request, #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875
# Which issue does this PR close?
Closes https://github.com/apache/arrow-datafusion/issues/2873
# Rationale for this change
Fixes a bug introduced in https://github.com/apache/arrow-datafusion/pull/2819
See https://github.com/apache/arrow-datafusion/issues/2873 (a query that used to work does not anymore). The underlying issue is described in https://github.com/apache/arrow-datafusion/issues/2874
# What changes are included in this PR?
Hack around https://github.com/apache/arrow-datafusion/issues/2874
# Are there any user-facing changes?
Sone queries on dictionaries work again
cc @viirya
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] tustvold commented on a diff in pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#discussion_r919188599
##########
datafusion/physical-expr/src/expressions/try_cast.rs:
##########
@@ -82,6 +82,19 @@ impl PhysicalExpr for TryCastExpr {
&array,
&self.cast_type,
)?)),
+ ColumnarValue::Scalar(scalar)
Review Comment:
Won't this prevent this from using the scalar comparison kernels, effectively reversing #2808
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2875: Fix casts of `ScalarValue::Utf8` to`DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#discussion_r918244473
##########
datafusion/physical-expr/src/expressions/try_cast.rs:
##########
@@ -184,10 +198,13 @@ mod tests {
// 3. evaluate the expression
// 4. verify that the resulting expression is of type B
// 5. verify that the resulting values are downcastable and correct
+ //
+ // $VALUE_FN is an expression (like `result.value`) that extracts the value at index `i`
macro_rules! generic_test_cast {
- ($A_ARRAY:ident, $A_TYPE:expr, $A_VEC:expr, $TYPEARRAY:ident, $TYPE:expr, $VEC:expr) => {{
+ ($A_ARRAY:ident, $A_TYPE:expr, $A_VEC:expr, $TYPEARRAY:ident, $TYPE:expr, $VEC:expr, $VALUE_FN:expr) => {{
Review Comment:
needed to add a way to extract the value of the result at element `i` because it is different for a `DictionaryArray`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#discussion_r919213066
##########
datafusion/physical-expr/src/expressions/try_cast.rs:
##########
@@ -82,6 +82,19 @@ impl PhysicalExpr for TryCastExpr {
&array,
&self.cast_type,
)?)),
+ ColumnarValue::Scalar(scalar)
Review Comment:
🤔 that is a good point -- perhaps I'll have a go at a proper fix for https://github.com/apache/arrow-datafusion/issues/2874
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb closed pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb closed pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
URL: https://github.com/apache/arrow-datafusion/pull/2875
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2875: Fix casts of `ScalarValue::Utf8` to`DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#discussion_r918244093
##########
datafusion/physical-expr/src/expressions/try_cast.rs:
##########
@@ -82,6 +82,19 @@ impl PhysicalExpr for TryCastExpr {
&array,
&self.cast_type,
)?)),
+ ColumnarValue::Scalar(scalar)
Review Comment:
This is the actual code change -- the rest of the PR is tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#issuecomment-1184876683
Closing in favor of https://github.com/apache/arrow-datafusion/pull/2891
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#issuecomment-1182023912
Will convert to draft as I attempt to "do it properly" as in https://github.com/apache/arrow-datafusion/issues/2874
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #2875: Fix casts of `ScalarValue::Utf8` to `DictionaryArray`
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2875:
URL: https://github.com/apache/arrow-datafusion/pull/2875#issuecomment-1182543735
Update: Doing it properly in https://github.com/apache/arrow-datafusion/pull/2891 is showing good promise
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org