Change default SQL mapping for `VARCAHR` from `Utf8` to `Utf8View` by zhuqi-lucas · Pull Request #16142 · apache/datafusion

zhuqi-lucas · 2025-05-22T06:33:21Z

Which issue does this PR close?

Closes#15096

Rationale for this change

Setting Varchar default mapping to utf8view, and change testing result.

What changes are included in this PR?

Setting Varchar default mapping to utf8view, and change testing result.

Are these changes tested?

Yes, new testing/slt testing result generated.

Are there any user-facing changes?

No, the internal datatype will change to utf8view.

…view

zhuqi-lucas · 2025-05-22T07:24:35Z

Interesting, sqllogic testing successfully for my local mac but CI failed...

…view

zhuqi-lucas · 2025-05-26T12:21:10Z

Resolved all problems now including the slt testing.

I think the PR is ready for benchmark progress to see if switching to Utf8View by default slows some queries down. cc @alamb Thanks a lot.

alamb · 2025-05-27T10:03:33Z

🤖 ./gh_compare_branch.shBenchmark Script Running
Linux aal-dev 6.11.0-1013-gcp #13~24.04.1-Ubuntu SMP Wed Apr 2 16:34:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing varchar_default_ut8view (5d9c537) to f3aed4adiff
Benchmarks: tpch_mem clickbench_partitioned clickbench_extended
Results will be posted here when complete

alamb · 2025-05-27T10:03:47Z

Resolved all problems now including the slt testing.
I think the PR is ready for benchmark progress to see if switching to Utf8View by default slows some queries down. cc @alamb Thanks a lot.

THanks @zhuqi-lucas -- I have started it up

alamb · 2025-05-27T10:43:34Z

🤖: Benchmark completed

Details

Comparing HEAD and varchar_default_ut8view
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query ┃ HEAD ┃ varchar_default_ut8view ┃ Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0 │ 1898.26ms │ 1810.38ms │ no change │
│ QQuery 1 │ 658.97ms │ 670.82ms │ no change │
│ QQuery 2 │ 1406.85ms │ 1396.48ms │ no change │
│ QQuery 3 │ 709.94ms │ 713.90ms │ no change │
│ QQuery 4 │ 1429.02ms │ 1398.58ms │ no change │
│ QQuery 5 │ 15171.65ms │ 15018.76ms │ no change │
│ QQuery 6 │ 2057.97ms │ 1992.46ms │ no change │
│ QQuery 7 │ 2336.53ms │ 2061.27ms │ +1.13x faster │
│ QQuery 8 │ 839.18ms │ 837.10ms │ no change │
└──────────────┴────────────┴─────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD) │ 26508.38ms │
│ Total Time (varchar_default_ut8view) │ 25899.76ms │
│ Average Time (HEAD) │ 2945.38ms │
│ Average Time (varchar_default_ut8view) │ 2877.75ms │
│ Queries Faster │ 1 │
│ Queries Slower │ 0 │
│ Queries with No Change │ 8 │
└────────────────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query ┃ HEAD ┃ varchar_default_ut8view ┃ Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0 │ 15.20ms │ 15.35ms │ no change │
│ QQuery 1 │ 33.35ms │ 33.29ms │ no change │
│ QQuery 2 │ 83.06ms │ 79.39ms │ no change │
│ QQuery 3 │ 96.03ms │ 95.15ms │ no change │
│ QQuery 4 │ 603.39ms │ 590.48ms │ no change │
│ QQuery 5 │ 851.82ms │ 855.46ms │ no change │
│ QQuery 6 │ 23.71ms │ 22.55ms │ no change │
│ QQuery 7 │ 36.83ms │ 37.07ms │ no change │
│ QQuery 8 │ 933.66ms │ 916.79ms │ no change │
│ QQuery 9 │ 1192.79ms │ 1193.28ms │ no change │
│ QQuery 10 │ 260.86ms │ 262.38ms │ no change │
│ QQuery 11 │ 299.90ms │ 293.48ms │ no change │
│ QQuery 12 │ 906.00ms │ 898.14ms │ no change │
│ QQuery 13 │ 1344.05ms │ 1220.93ms │ +1.10x faster │
│ QQuery 14 │ 842.98ms │ 823.52ms │ no change │
│ QQuery 15 │ 842.44ms │ 835.95ms │ no change │
│ QQuery 16 │ 1725.19ms │ 1718.98ms │ no change │
│ QQuery 17 │ 1598.00ms │ 1614.71ms │ no change │
│ QQuery 18 │ 3092.64ms │ 3093.01ms │ no change │
│ QQuery 19 │ 81.09ms │ 81.79ms │ no change │
│ QQuery 20 │ 1111.88ms │ 1115.69ms │ no change │
│ QQuery 21 │ 1298.42ms │ 1296.34ms │ no change │
│ QQuery 22 │ 2165.31ms │ 2174.91ms │ no change │
│ QQuery 23 │ 8034.33ms │ 7921.79ms │ no change │
│ QQuery 24 │ 458.46ms │ 457.26ms │ no change │
│ QQuery 25 │ 391.76ms │ 372.64ms │ no change │
│ QQuery 26 │ 528.56ms │ 519.61ms │ no change │
│ QQuery 27 │ 1595.08ms │ 1586.78ms │ no change │
│ QQuery 28 │ 12566.00ms │ 12405.74ms │ no change │
│ QQuery 29 │ 535.24ms │ 532.68ms │ no change │
│ QQuery 30 │ 796.06ms │ 807.39ms │ no change │
│ QQuery 31 │ 863.55ms │ 857.96ms │ no change │
│ QQuery 32 │ 2633.13ms │ 2621.41ms │ no change │
│ QQuery 33 │ 3300.20ms │ 3347.80ms │ no change │
│ QQuery 34 │ 3429.42ms │ 3335.38ms │ no change │
│ QQuery 35 │ 1338.57ms │ 1340.32ms │ no change │
│ QQuery 36 │ 123.48ms │ 118.34ms │ no change │
│ QQuery 37 │ 55.36ms │ 56.67ms │ no change │
│ QQuery 38 │ 119.30ms │ 117.49ms │ no change │
│ QQuery 39 │ 191.24ms │ 187.82ms │ no change │
│ QQuery 40 │ 46.61ms │ 47.57ms │ no change │
│ QQuery 41 │ 45.34ms │ 44.25ms │ no change │
│ QQuery 42 │ 39.10ms │ 38.06ms │ no change │
└──────────────┴────────────┴─────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD) │ 56529.39ms │
│ Total Time (varchar_default_ut8view) │ 55985.60ms │
│ Average Time (HEAD) │ 1314.64ms │
│ Average Time (varchar_default_ut8view) │ 1301.99ms │
│ Queries Faster │ 1 │
│ Queries Slower │ 0 │
│ Queries with No Change │ 42 │
└────────────────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query ┃ HEAD ┃ varchar_default_ut8view ┃ Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 1 │ 122.52ms │ 121.90ms │ no change │
│ QQuery 2 │ 22.09ms │ 22.25ms │ no change │
│ QQuery 3 │ 33.67ms │ 34.12ms │ no change │
│ QQuery 4 │ 19.78ms │ 19.82ms │ no change │
│ QQuery 5 │ 50.90ms │ 52.52ms │ no change │
│ QQuery 6 │ 11.94ms │ 11.89ms │ no change │
│ QQuery 7 │ 94.09ms │ 93.62ms │ no change │
│ QQuery 8 │ 25.99ms │ 25.51ms │ no change │
│ QQuery 9 │ 60.34ms │ 59.78ms │ no change │
│ QQuery 10 │ 56.02ms │ 56.36ms │ no change │
│ QQuery 11 │ 11.44ms │ 11.54ms │ no change │
│ QQuery 12 │ 41.27ms │ 41.51ms │ no change │
│ QQuery 13 │ 27.81ms │ 27.49ms │ no change │
│ QQuery 14 │ 9.53ms │ 9.54ms │ no change │
│ QQuery 15 │ 23.84ms │ 24.62ms │ no change │
│ QQuery 16 │ 21.35ms │ 22.00ms │ no change │
│ QQuery 17 │ 95.29ms │ 95.13ms │ no change │
│ QQuery 18 │ 207.37ms │ 212.10ms │ no change │
│ QQuery 19 │ 26.88ms │ 26.29ms │ no change │
│ QQuery 20 │ 38.11ms │ 37.17ms │ no change │
│ QQuery 21 │ 164.65ms │ 161.44ms │ no change │
│ QQuery 22 │ 15.92ms │ 16.45ms │ no change │
└──────────────┴──────────┴─────────────────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD) │ 1180.80ms │
│ Total Time (varchar_default_ut8view) │ 1183.07ms │
│ Average Time (HEAD) │ 53.67ms │
│ Average Time (varchar_default_ut8view) │ 53.78ms │
│ Queries Faster │ 0 │
│ Queries Slower │ 0 │
│ Queries with No Change │ 22 │
└────────────────────────────────────────┴───────────┘

zhuqi-lucas · 2025-05-27T10:49:13Z

Thank you @alamb , it seems no regression from the benchmark result!

…view

alamb

Thank you @zhuqi-lucas -- this is great

FYI @jayzhan211 , @timsaucer @xudong963 and @comphead

I have a few suggestions on how to simplify the examples which would be nice to get in prior to merge but I don't think it is needed

alamb · 2025-05-27T16:14:40Z

+
 // Assuming the input columns are
- // column[0]: customer_id / UTF8
+ // column[0]: customer_id / UTF8 or UTF8View


I feel like the example would be simpler if it just used Utf8View. Could we do that and avoid adding the logic to handle both types?

Good suggestion! Thank you @alamb , addressed in latest PR.

alamb · 2025-05-27T16:15:20Z

 # specific language governing permissions and limitations
 # under the License.

+# Currently, the avro not support Utf8View type, so we disable the map_varchar_to_utf8view


alamb · 2025-05-27T16:17:11Z

There appears to be an update of parquet-testing with unrelated content, but I think it is ok to update it too

alamb · 2025-05-27T16:17:51Z


 // Ensure the column names and types match the target table
- df = df.with_column_renamed("column1", "tablecol1").unwrap();
+ df = df


I think it would be better if we could change the example to maybe just use a StringViewArray in the first place (rather than have to cast it here)

Good suggestion! Thank you @alamb , addressed in latest PR.

zhuqi-lucas · 2025-05-28T03:33:03Z

Thank you @zhuqi-lucas -- this is great
FYI @jayzhan211 , @timsaucer @xudong963 and @comphead
I have a few suggestions on how to simplify the examples which would be nice to get in prior to merge but I don't think it is needed

Thank you @alamb for review, i also addressed the suggestions about example in latest PR.

alamb · 2025-05-28T15:08:58Z

I'll plan to merge this tomorrow unless I hear anything different

…view

xudong963 · 2025-05-29T13:28:29Z

I'm reviewing

xudong963

LGTM

zhuqi-lucas · 2025-05-29T15:24:32Z

Thank you @xudong963 for review!

xudong963 · 2025-05-30T05:44:13Z

Let's go, thank you all

alamb · 2025-05-30T13:15:56Z

Thanks again @zhuqi-lucas and @xudong963

I have also made a PR to add a mention of this change to the upgrade guide:

Add change to VARCHAR in the upgrade guide #16216

zhuqi-lucas added 6 commits May 21, 2025 21:06

Change default mapping of SQL VARCHAR from Utf8 to Utf8View
cd288b9

Merge remote-tracking branch 'upstream/main' into varchar_default_ut8…
2ec5370
…view

Fix test
f2e9963

Fix doc
83fe677

Add utf8view support for user defined
0be56a3

Merge remote-tracking branch 'upstream/main' into varchar_default_ut8…
18c83db
…view

zhuqi-lucas marked this pull request as draft May 22, 2025 06:33

github-actionsBot added documentation Improvements or additions to documentation sql SQL Planner core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) common Related to common crate labels May 22, 2025

zhuqi-lucas added 2 commits May 22, 2025 16:16

update testing data
0bb1d96

Merge remote-tracking branch 'upstream/main' into varchar_default_ut8…
0780263
…view

zhuqi-lucas force-pushed the varchar_default_ut8view branch from 6ee857a to 0780263Compare May 22, 2025 08:17

zhuqi-lucas added 9 commits May 24, 2025 23:19

Merge remote-tracking branch 'upstream/main' into varchar_default_ut8…
c8295af
…view

fix
3a66192

clippy
f44da69

fix
a5eaa28

Fix
805d99e

Fix fmt
d806b8c

Fix
afac8c8

Fix slt testing
4fe5414

add doc for avro utf8view
02d725a

zhuqi-lucas marked this pull request as ready for review May 26, 2025 12:14

zhuqi-lucas changed the title ~~POC Varchar default mapping to utf8view~~feat: varchar default mapping to utf8viewMay 26, 2025

zhuqi-lucas mentioned this pull request May 26, 2025
Change mapping of SQL VARCHAR from Utf8 to Utf8View#15096
Closed
6 tasks

Support utf8view for STRING_AGG
2dc6bc5

Merge remote-tracking branch 'upstream/main' into varchar_default_ut8…
e54e432
…view

zhuqi-lucas force-pushed the varchar_default_ut8view branch from 5d9c537 to e54e432Compare May 27, 2025 10:54

github-actionsBot added the functions Changes to functions implementation label May 27, 2025

alamb added the api change Changes the API exposed to users of the crate label May 27, 2025

alamb mentioned this pull request May 27, 2025
Release DataFusion 48.0.0 (June 2025) #15771
Closed
29 tasks

alamb approved these changes May 27, 2025
View reviewed changes

alamb changed the title ~~feat: varchar default mapping to utf8view~~Change default SQL mapping for VARCAHR from Utf8 to Utf8ViewMay 27, 2025

Address comments
65e0794

fmt
b3bb360

Merge remote-tracking branch 'upstream/main' into varchar_default_ut8…
9195b6a
…view

zhuqi-lucas force-pushed the varchar_default_ut8view branch from e7243d4 to 9195b6aCompare May 29, 2025 10:18

xudong963 approved these changes May 29, 2025
View reviewed changes

xudong963 merged commit 21248fb into apache:mainMay 30, 2025

alamb mentioned this pull request May 30, 2025
Add change to VARCHAR in the upgrade guide #16216
Merged

Uh oh!

Conversation

zhuqi-lucas commented May 22, 2025• edited 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!

zhuqi-lucas commented May 22, 2025

Uh oh!

zhuqi-lucas commented May 26, 2025

Uh oh!

alamb commented May 27, 2025

Uh oh!

alamb commented May 27, 2025

Uh oh!

alamb commented May 27, 2025

Uh oh!

zhuqi-lucas commented May 27, 2025

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alambMay 27, 2025

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucasMay 28, 2025

Choose a reason for hiding this comment

Uh oh!

alambMay 27, 2025

Choose a reason for hiding this comment

Uh oh!

alambMay 27, 2025

Choose a reason for hiding this comment

Uh oh!

alambMay 27, 2025

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucasMay 28, 2025

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucas commented May 28, 2025

Uh oh!

alamb commented May 28, 2025

Uh oh!

xudong963 commented May 29, 2025

Uh oh!

xudong963 left a comment

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucas commented May 29, 2025

Uh oh!

xudong963 commented May 30, 2025

Uh oh!

alamb commented May 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhuqi-lucas commented May 22, 2025•
edited
Loading