jayzhan211 commented on code in PR #25874:
URL: https://github.com/apache/datafusion/pull/25874#discussion_r4366366373
##########
datafusion/functions/src/unicode/substr.rs:
##########
@@ -73,7 +83,8 @@ impl Default for SubstrFunc {
impl SubstrFunc {
pub fn new() -> Self {
- let string =
Coercion::new_exact(TypeSignatureClass::Native(logical_string()));
+ let string =
Coercion::new_exact(TypeSignatureClass::Native(logical_string()))
+ .with_encoding_preservation(EncodingPreservation::dictionary());
Review Comment:
I dug into this further. Part of the cost was hashing and dictionary merging
doing work for every dictionary value instead of every row. I'll fix that in
the shared code rather than in the aggregate (see #26696), and with a prototype
of that this query goes from 1.20x to 1.12x of main. The rest is the general
cost of dictionary group keys (#24822), which `lower`/`btrim` on dictionary
columns already pay on main, so I don't think it needs to block this PR. The
guard on the fast path is still needed for Q22.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
