You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/09/05 21:03:43 UTC
Re: [PR] Coerce BinaryView/Utf8View to LargeBinary/LargeUtf8 on output. [datafusion]
alamb commented on code in PR #12271:
URL: https://github.com/apache/datafusion/pull/12271#discussion_r1746147461
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -982,6 +1056,128 @@ mod test {
assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), plan, expected)
}
+ #[test]
+ fn coerce_utf8view_output() -> Result<()> {
+ let expr = col("a");
+ let empty = empty_with_type(DataType::Utf8View);
+ let plan = LogicalPlan::Projection(Projection::try_new(
+ vec![expr.clone()],
+ Arc::clone(&empty),
+ )?);
+
+ // baseline
+ let expected = "Projection: a\n EmptyRelation";
+ assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), plan.clone(), expected)?;
+
+ // coerce: Utf8View => LargeUtf8
+ let expect_cast = "Projection: CAST(a AS LargeUtf8)\n EmptyRelation";
+ let mut options = ConfigOptions::default();
+ options.optimizer.expand_views_at_output = true;
+ assert_analyzed_plan_with_config_eq(
+ options,
+ Arc::new(TypeCoercion::new()),
+ plan.clone(),
+ expect_cast,
+ )?;
+
+ // request coerce --> but output in bool, not Utf8View
+ let bool_expr = col("a").lt(lit("foo"));
+ let plan_no_cast = LogicalPlan::Projection(Projection::try_new(
+ vec![bool_expr],
+ Arc::clone(&empty),
+ )?);
+ let expect_no_cast =
Review Comment:
I found the name with `no_cast` here confusing -- this predicate does actually have a cast (just as part of the inner expression)
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -514,6 +534,59 @@ impl<'a> TreeNodeRewriter for TypeCoercionRewriter<'a> {
}
}
+/// Transform a schema to use non-view types for Utf8View and BinaryView
+fn transform_schema_to_nonview(dfschema: &DFSchemaRef) -> Option<Result<DFSchema>> {
+ let metadata = dfschema.as_arrow().metadata.clone();
+ let len = dfschema.fields().len();
+ let mut transformed = false;
+
+ let (qualifiers, transformed_fields) = dfschema
+ .iter()
+ .map(|(qualifier, field)| match field.data_type() {
+ DataType::Utf8View => {
+ transformed = true;
+ (
+ qualifier,
+ Arc::new(Field::new(
+ field.name(),
+ DataType::LargeUtf8,
+ field.is_nullable(),
+ )),
+ )
+ }
+ DataType::BinaryView => {
+ transformed = true;
+ (
+ qualifier,
+ Arc::new(Field::new(
+ field.name(),
+ DataType::LargeBinary,
+ field.is_nullable(),
+ )),
+ )
+ }
+ _ => (qualifier, Arc::clone(field)),
+ })
+ .fold(
+ (Vec::with_capacity(len), Vec::with_capacity(len)),
+ |(mut qs, mut fs), (q, f)| {
+ qs.push(q.cloned());
+ fs.push(f);
+ (qs, fs)
+ },
+ );
Review Comment:
I think this looks like `std::iterator::unzip`: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.unzip
untested:
```suggestion
.unzip();
```
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -982,6 +1056,128 @@ mod test {
assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), plan, expected)
}
+ #[test]
+ fn coerce_utf8view_output() -> Result<()> {
+ let expr = col("a");
+ let empty = empty_with_type(DataType::Utf8View);
+ let plan = LogicalPlan::Projection(Projection::try_new(
+ vec![expr.clone()],
+ Arc::clone(&empty),
+ )?);
+
+ // baseline
+ let expected = "Projection: a\n EmptyRelation";
Review Comment:
It might be nice to create some sort of function / struct that avoids the repetition in the tests (and thus makes the intent of the test easier to understand
Maybe something like
```rust
expect_coerced_plan(expected, plan, options)
```
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -982,6 +1056,128 @@ mod test {
assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), plan, expected)
}
+ #[test]
+ fn coerce_utf8view_output() -> Result<()> {
+ let expr = col("a");
+ let empty = empty_with_type(DataType::Utf8View);
+ let plan = LogicalPlan::Projection(Projection::try_new(
+ vec![expr.clone()],
+ Arc::clone(&empty),
+ )?);
+
+ // baseline
+ let expected = "Projection: a\n EmptyRelation";
+ assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), plan.clone(), expected)?;
+
+ // coerce: Utf8View => LargeUtf8
+ let expect_cast = "Projection: CAST(a AS LargeUtf8)\n EmptyRelation";
+ let mut options = ConfigOptions::default();
+ options.optimizer.expand_views_at_output = true;
+ assert_analyzed_plan_with_config_eq(
+ options,
+ Arc::new(TypeCoercion::new()),
+ plan.clone(),
+ expect_cast,
+ )?;
+
+ // request coerce --> but output in bool, not Utf8View
+ let bool_expr = col("a").lt(lit("foo"));
+ let plan_no_cast = LogicalPlan::Projection(Projection::try_new(
+ vec![bool_expr],
+ Arc::clone(&empty),
+ )?);
+ let expect_no_cast =
+ "Projection: a < CAST(Utf8(\"foo\") AS Utf8View)\n EmptyRelation";
+ let mut options = ConfigOptions::default();
+ options.optimizer.expand_views_at_output = true;
+ assert_analyzed_plan_with_config_eq(
+ options,
+ Arc::new(TypeCoercion::new()),
+ plan_no_cast.clone(),
+ expect_no_cast,
+ )?;
+
+ // coerce start with a non-projection root logical plan node
+ let sort_expr = expr.sort(true, true);
+ let sort_plan = LogicalPlan::Sort(Sort {
+ expr: vec![sort_expr],
+ input: Arc::new(plan),
+ fetch: None,
+ });
+ let expect_cast = "Projection: CAST(a AS LargeUtf8)\n Sort: a ASC NULLS FIRST\n Projection: a\n EmptyRelation";
+ let mut options = ConfigOptions::default();
+ options.optimizer.expand_views_at_output = true;
+ assert_analyzed_plan_with_config_eq(
Review Comment:
Could you also please add a test with two levels of projections (to ensure only the top most one is coerced)?
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org