feat: mapping sql Char/Text/String default to Utf8View #16290

zhuqi-lucas · 2025-06-06T09:39:24Z

Which issue does this PR close?

Closes Mapping Char/Text/String default to Utf8View #16288

Rationale for this change

Similar to

#15096

But most work we have done, so it's easier for us to finish this ticket.

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

zhuqi-lucas · 2025-06-08T08:16:46Z

datafusion/expr-common/src/type_coercion/binary.rs

@@ -1202,7 +1202,7 @@ pub fn string_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataT
 fn numeric_string_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
    use arrow::datatypes::DataType::*;
    match (lhs_type, rhs_type) {
-        (Utf8 | LargeUtf8, other_type) | (other_type, Utf8 | LargeUtf8)
+        (Utf8 | LargeUtf8 | Utf8View, other_type) | (other_type, Utf8 | LargeUtf8 | Utf8View)


This change fix the bug, Utf8View with other numeric type can't coercion.

zhuqi-lucas · 2025-06-08T14:17:51Z

datafusion/sqllogictest/test_files/arrow_files.slt

@@ -61,22 +61,12 @@ LOCATION '../core/tests/data/partitioned_table_arrow/'
 PARTITIONED BY (part);

 # select wildcard
-query ITBI
+query error DataFusion error: Arrow error: External error: Arrow error: Invalid argument error: column types must match schema types, expected Utf8View but found Utf8 at column index 1


cc @xudong963 @alamb , this is the only remaining issue, do you know why Arrow format do not support Utf8 with Utf8View? Thanks!

This looks to me like there is a mismatch between what is declared in the plan and what the actual types are. Are you able to figure out what the stack trace is that throws this error?

Thank you @alamb , after investigate i believe i found the root cause:

We using fixed arrow file to test for sqllogictests, and this arrow field is writing with arrow-ipc utf8, so when we decode to read it's also loading utf8. But we default the field for sql to mapping to utf8view for this PR, so when we create the record batch we add checking default, it will failed.

zhuqi-lucas · 2025-06-11T07:16:23Z

Hi @alamb @xudong963 , i have fixed all the corner cases in this PR, it's ready for review! Thanks!

xudong963 · 2025-06-11T09:05:08Z

datafusion/common/src/config.rs

        pub map_varchar_to_utf8view: bool, default = true

+        /// If true, `CHAR` and `Text` and `String` is mapped to `Utf8View` during SQL planning.
+        /// If false, `CHAR` and `Text` and `String` is mapped to `Utf8` during SQL planning.
+        /// Default is true.
+        pub map_char_to_utf8view: bool, default = true
+


Do we need the two configs? I mean, if we can directly use one.

Imagine that one is true, and another is false, does it make sense? Before we didn't have utf8view, varchar,char, text, string all will be utf8, they're consistent.

Thank you @xudong963 for review, this is true, we can use one config, maybe the name should be:

map_sql_string_to_utf8view for unifed all sql string to utf8view

How about:

/// If true, string types (VARCHAR, CHAR, Text, and String) are mapped to `Utf8View` during SQL planning. /// If false, they are mapped to `Utf8`. /// Default is true. pub map_string_types_to_utf8view: bool, default = true

Also cc @alamb

How about:

/// If true, string types (VARCHAR, CHAR, Text, and String) are mapped to `Utf8View` during SQL planning. /// If false, they are mapped to `Utf8`. /// Default is true. pub map_string_types_to_utf8view: bool, default = true

Also cc @alamb

Thanks @xudong963 looks good to me.

Addressed in latest PR, thanks!

xudong963 · 2025-06-16T12:27:13Z

Some tests are failing

zhuqi-lucas · 2025-06-16T13:37:10Z

Some tests are failing

Fixed in latest PR, thank you @xudong963 !

alamb

Thank you @zhuqi-lucas and @xudong963

alamb · 2025-06-16T21:50:42Z

datafusion/common/src/config.rs

+        /// If true, string types (VARCHAR, CHAR, Text, and String) are mapped to `Utf8View` during SQL planning.
+        /// If false, they are mapped to `Utf8`.
+        /// Default is true.
+        pub map_string_types_to_utf8view: bool, default = true


I think this is an API change, so perhaps we can add a note about this in the upgrade guide (that we changed the name of the config setting)

Thank you @alamb for review, i agree we need to add this to the upgrade guide for next release.

Let's file a ticket to record it

Created the ticket:

#16428

alamb · 2025-06-16T21:51:58Z

datafusion/sqllogictest/test_files/array.slt

@@ -6082,7 +6082,7 @@ physical_plan
 04)------AggregateExec: mode=Partial, gby=[], aggr=[count(Int64(1))]
 05)--------ProjectionExec: expr=[]
 06)----------CoalesceBatchesExec: target_batch_size=8192
-07)------------FilterExec: substr(md5(CAST(value@0 AS Utf8)), 1, 32) IN ([Literal { value: Utf8View("7f4b18de3cfeb9b4ac78c381ee2ad278"), field: Field { name: "7f4b18de3cfeb9b4ac78c381ee2ad278", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("a"), field: Field { name: "a", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("b"), field: Field { name: "b", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("c"), field: Field { name: "c", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }])
+07)------------FilterExec: substr(CAST(md5(CAST(value@0 AS Utf8View)) AS Utf8View), 1, 32) IN ([Literal { value: Utf8View("7f4b18de3cfeb9b4ac78c381ee2ad278"), field: Field { name: "7f4b18de3cfeb9b4ac78c381ee2ad278", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("a"), field: Field { name: "a", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("b"), field: Field { name: "b", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }, Literal { value: Utf8View("c"), field: Field { name: "c", data_type: Utf8View, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} } }])


🤔 this looks like it may have added extra casts I wonder if it because md5 doesn't support StringView natively 🤔

Good catch, thank you @alamb , i support it now in latest PR.

feat: mapping sql Char/Text/String default to Utf8View

14bd404

zhuqi-lucas marked this pull request as draft June 6, 2025 09:39

github-actions bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels Jun 6, 2025

zhuqi-lucas mentioned this pull request Jun 6, 2025

Question about the map_varchar_to_utf8view config #16277

Closed

Add support utf8view for sort merge join

4973bf9

github-actions bot added core Core DataFusion crate physical-plan Changes to the physical-plan crate labels Jun 6, 2025

zhuqi-lucas marked this pull request as ready for review June 6, 2025 09:53

zhuqi-lucas marked this pull request as draft June 6, 2025 09:58

fix binary utf8view union with int32

85bf391

github-actions bot added the logical-expr Logical plan and expressions label Jun 8, 2025

zhuqi-lucas commented Jun 8, 2025

View reviewed changes

fix slt order

406575b

zhuqi-lucas marked this pull request as ready for review June 8, 2025 11:54

zhuqi-lucas added 4 commits June 8, 2025 21:22

Merge remote-tracking branch 'upstream/main' into issue_16288

bcb5645

fix

8ad6491

fmt

0b4ee41

Fix test

8ee72c9

zhuqi-lucas commented Jun 8, 2025

View reviewed changes

fix

0b05cb6

github-actions bot added documentation Improvements or additions to documentation common Related to common crate labels Jun 11, 2025

zhuqi-lucas added 4 commits June 11, 2025 13:46

Merge remote-tracking branch 'upstream/main' into issue_16288

e4560a1

fix test

d825fd1

fix

f341e1e

clean

6485228

xudong963 reviewed Jun 11, 2025

View reviewed changes

Address comments

fd20268

Merge remote-tracking branch 'upstream/main' into issue_16288

a715d10

zhuqi-lucas added 2 commits June 16, 2025 20:53

Merge remote-tracking branch 'upstream/main' into issue_16288

15aba27

Fix test

9802850

alamb approved these changes Jun 16, 2025

View reviewed changes

zhuqi-lucas added 2 commits June 17, 2025 11:15

support md5 for ut8view

1187834

Merge remote-tracking branch 'upstream/main' into issue_16288

69838fd

github-actions bot added the functions Changes to functions implementation label Jun 17, 2025

xudong963 approved these changes Jun 17, 2025

View reviewed changes

xudong963 merged commit 06631c2 into apache:main Jun 17, 2025
29 checks passed

zhuqi-lucas mentioned this pull request Jun 17, 2025

Update upgrade md for new unified config for sql string mapping to utf8view when we release datafusion 49.0.0 #16428

Open

feat: mapping sql Char/Text/String default to Utf8View #16290

feat: mapping sql Char/Text/String default to Utf8View #16290

Uh oh!

Conversation

zhuqi-lucas commented Jun 6, 2025 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucas Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucas commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucas Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xudong963 commented Jun 16, 2025

Uh oh!

zhuqi-lucas commented Jun 16, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zhuqi-lucas commented Jun 6, 2025 •

edited by alamb

Loading

zhuqi-lucas Jun 11, 2025 •

edited

Loading

zhuqi-lucas Jun 11, 2025 •

edited

Loading