You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/09/05 20:01:59 UTC
[GitHub] [arrow-rs] alamb opened a new issue, #4781: DynScalar abstraction (or something that makes it easy to create scalar values)
alamb opened a new issue, #4781:
URL: https://github.com/apache/arrow-rs/issues/4781
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
There is code like this in IOx that compare a value to `0`
https://github.com/influxdata/influxdb_iox/blob/2a71fcbc76a695b1129895932746084a0a258bab/iox_query_influxql/src/window/non_negative.rs#L59-L60
This uses the now deprecated [`ly_dyn_scalar`](https://docs.rs/arrow/latest/arrow/compute/kernels/comparison/fn.lt_dyn_scalar.html) kernel
I would like to update to use the non deprecated kernel `lt`: https://docs.rs/arrow/latest/arrow/compute/kernels/cmp/fn.lt.html
However, to do so requires a "Datum" and thus I need some way to create a typed `Datum` for zero
**Describe the solution you'd like**
I don't really have a strong opinion, I just want something that will work reasonably for this usecase.
**Describe alternatives you've considered**
I found that arrow crate uses a function and [`Scalar`](https://docs.rs/arrow-array/46.0.0/arrow_array/struct.Scalar.html) to do this: https://docs.rs/arrow-ord/46.0.0/src/arrow_ord/comparison.rs.html#36-221 however the `Scalar` appears to be strongly typed which makes it hard to use with a dynamically typed kernel that takes `&dyn Datum` (aka is "type erased") without changing all callsites.
Here is one possibility to perhaps expose the code that Arrow uses internally:
```rust
/// A dynamic Datum that wraps an array and represents a single value
#[derive(Debug, Clone)]
pub struct ScalarDatum
{
arr: ArrayRef,
}
impl ScalarDatum {
/// Create a [`ScalarDatum`] of type `d` that represents the primtive scalar value `scalar`
fn try_new_primitive<T: num::ToPrimitive + std::fmt::Debug>(
d: &DataType,
scalar: T,
) -> Result<Self, ArrowError> {
Ok(Self {
arr: make_primitive_scalar_array(d, scalar)?,
})
}
}
impl Datum for ScalarDatum {
...
}
```
**Additional context**
<!--
Add any other context or screenshots about the feature request here.
-->
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1710232726
Update here is that I could use `ScalarValue::to_scalar` successfully
So instead of
```rust
let predicate = lt_dyn_scalar(&array, 0)?;
```
I did:
```rust
let zero = ScalarValue::new_zero(array.data_type())?;
let predicate = lt(&array, &zero.to_scalar())?;
```
Which seems to have worked nicely
--
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-rs] alamb commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1707292337
> FWIW this could just be Scalar<ArrayRef>
That is good to know -- I did not realize that. Perhaps I will make a PR with some additional documentation about it in arrow-rs
> Also not a great solution but you could just use ScalarValue::new_zero 😅
That would be just fine for this usecase -- but how can I use that? `ScalarValue` doesn't appear to implement `Datum` 🤔
https://docs.rs/datafusion/latest/datafusion/common/enum.ScalarValue.html
Or perhaps you are saying call https://docs.rs/datafusion/latest/datafusion/common/enum.ScalarValue.html#method.to_array and then make a `Scalar<ArrayRef>` 🤔 that could work
--
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-rs] alamb commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1707256175
> This appears to be reinstating an API that was explicitly deprecated and removed... https://github.com/apache/arrow-rs/issues/2837
I would phrase it as "the deprecated API has a real usecase which the new API does not address"
If there is some reason this shouldn't exist in arrow, I think what will happen is downstream crates will implement it (I was actually going to do so in IOx but I got stuck on https://github.com/apache/arrow-rs/issues/4780).
I am not sure what the concern about a dynamic scalar is given the kernels are now dynamically typed
--
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-rs] tustvold commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1707293878
> Or perhaps you are saying call
I believe there is a to_scalar method, possibly not released yet
--
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-rs] alamb commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1710267472
Added some doc comments in DataFusion to make it easier to find: https://github.com/apache/arrow-datafusion/pull/7491
--
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-rs] tustvold commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1707252571
This appears to be reinstating an API that was explicitly deprecated and removed... #2837
--
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-rs] alamb commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1710412297
> FWIW this could just be Scalar<ArrayRef>
Here is a PR that improves the docs to encode this (it was what I was missing when I first filed this, I think): https://github.com/apache/arrow-rs/pull/4793
--
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-rs] tustvold commented on issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4781:
URL: https://github.com/apache/arrow-rs/issues/4781#issuecomment-1707284805
> dynamic scalar is given the kernels are now dynamically typed
It is the slightly funny type coercion that makes me uncomfortable, it seems strange to me that I'd be able to create a TimestampNanosecondArray from a float, or a u64, etc... _A lesser point is that this won't work for Decimal256._ I don't have a good solution here, I can appreciate the importance of making it easy to create scalars representing zero, and possibly other values like 1 or -1, I'm just not sure how best to express this
> pub struct ScalarDatum
FWIW this could just be `Scalar<ArrayRef>`
> https://github.com/influxdata/influxdb_iox/blob/2a71fcbc76a695b1129895932746084a0a258bab/iox_query_influxql/src/window/non_negative.rs#L59-L60
I appreciate this is not a general solution, but in this particular case we can see two lines above that there are only two possible output types, which could simply be enumerated. This would also likely be significantly faster.
Also not a great solution but you could just use `ScalarValue::new_zero` :sweat_smile:
--
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-rs] alamb closed issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #4781: `DynScalar` abstraction (something that makes it easy to create scalar `Datum`s)
URL: https://github.com/apache/arrow-rs/issues/4781
--
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