alamb commented on code in PR #2891:
URL: https://github.com/apache/arrow-datafusion/pull/2891#discussion_r922068389
##########
datafusion/common/src/scalar.rs:
##########
@@ -971,7 +1038,54 @@ impl ScalarValue {
Arc::new(StructArray::from(field_values))
}
- _ => {
+ DataType::Dictionary(key_type, value_type) => {
+ // create the values array
+ let value_scalars = scalars
+ .into_iter()
+ .map(|scalar| match scalar {
+ ScalarValue::Dictionary(inner_key_type, scalar) => {
+ if &inner_key_type == key_type {
+ Ok(*scalar)
+ } else{
+ panic!("Expected inner key type of {} but
found: {}, value was ({:?})", key_type, inner_key_type, scalar);
+ }
+ },
+ _ => {
+ Err(DataFusionError::Internal(format!(
+ "Expected scalar of type {} but found: {}
{:?}",
+ value_type, scalar, scalar
+ )))
+ },
+ })
+ .collect::<Result<Vec<_>>>()?;
+
+ let values = Self::iter_to_array(value_scalars)?;
+ assert_eq!(values.data_type(), value_type.as_ref());
+
+ match key_type.as_ref() {
+ DataType::Int8 => dict_from_values::<Int8Type>(&values)?,
+ DataType::Int16 => dict_from_values::<Int16Type>(&values)?,
+ DataType::Int32 => dict_from_values::<Int32Type>(&values)?,
+ DataType::Int64 => dict_from_values::<Int64Type>(&values)?,
+ DataType::UInt8 => dict_from_values::<UInt8Type>(&values)?,
+ DataType::UInt16 =>
dict_from_values::<UInt16Type>(&values)?,
+ DataType::UInt32 =>
dict_from_values::<UInt32Type>(&values)?,
+ DataType::UInt64 =>
dict_from_values::<UInt64Type>(&values)?,
+ _ => unreachable!("Invalid dictionary keys type: {:?}",
key_type),
+ }
+ }
+ // explicitly enumerate unsupported types so newly added
Review Comment:
It took me a while to find that `ScalarValue::Dictionary` was missing here
due to the default `_` match.
##########
datafusion/common/src/scalar.rs:
##########
@@ -101,6 +101,8 @@ pub enum ScalarValue {
IntervalMonthDayNano(Option<i128>),
/// struct of nested ScalarValue
Struct(Option<Vec<ScalarValue>>, Box<Vec<Field>>),
+ /// Dictionary type: index type and value
+ Dictionary(Box<DataType>, Box<ScalarValue>),
Review Comment:
The core change is to introduce this variant and the rest of the PR is
implementing it as well as removing the partial support for Dictionaries that
existed previously
##########
datafusion/common/src/scalar.rs:
##########
@@ -3159,4 +3326,42 @@ mod tests {
DataType::Timestamp(TimeUnit::Nanosecond, Some("UTC".to_owned()))
);
}
+
+ #[test]
+ fn cast_round_trip() {
+ check_scalar_cast(ScalarValue::Int8(Some(5)), DataType::Int16);
+ check_scalar_cast(ScalarValue::Int8(None), DataType::Int16);
+
+ check_scalar_cast(ScalarValue::Float64(Some(5.5)), DataType::Int16);
+
+ check_scalar_cast(ScalarValue::Float64(None), DataType::Int16);
+
+ check_scalar_cast(
Review Comment:
This test would fail before this change
##########
datafusion/core/tests/sql/projection.rs:
##########
@@ -220,6 +221,48 @@ async fn preserve_nullability_on_projection() ->
Result<()> {
Ok(())
}
+#[tokio::test]
+async fn project_cast_dictionary() {
Review Comment:
Reproducer for https://github.com/apache/arrow-datafusion/issues/2873
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
