Re: [PR] Always use `StringViewArray` as output of `substr` [datafusion]

[via GitHub]

2010YOUY01 commented on PR #14498:
URL: https://github.com/apache/datafusion/pull/14498#issuecomment-2644818671

   > My benchmark results show no change in Q22 (but also clearly the data is 
quite noisy 🤔 )
   > 
   > ```
   > --------------------
   > Benchmark tpch_sf1.json
   > --------------------
   > 
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
   > ┃ Query        ┃ main_base ┃ substr-always-output-utf8view ┃        Change 
┃
   > 
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
   > │ QQuery 1     │  220.66ms │                      217.19ms │     no change 
│
   > │ QQuery 2     │  120.04ms │                      118.99ms │     no change 
│
   > │ QQuery 3     │  117.00ms │                      133.73ms │  1.14x slower 
│
   > │ QQuery 4     │   82.79ms │                       84.31ms │     no change 
│
   > │ QQuery 5     │  161.05ms │                      158.10ms │     no change 
│
   > │ QQuery 6     │   43.91ms │                       61.73ms │  1.41x slower 
│
   > │ QQuery 7     │  196.49ms │                      208.38ms │  1.06x slower 
│
   > │ QQuery 8     │  161.86ms │                      168.99ms │     no change 
│
   > │ QQuery 9     │  239.92ms │                      240.70ms │     no change 
│
   > │ QQuery 10    │  207.67ms │                      203.80ms │     no change 
│
   > │ QQuery 11    │   96.42ms │                       94.76ms │     no change 
│
   > │ QQuery 12    │  104.79ms │                      119.54ms │  1.14x slower 
│
   > │ QQuery 13    │  203.66ms │                      208.01ms │     no change 
│
   > │ QQuery 14    │   71.92ms │                       70.81ms │     no change 
│
   > │ QQuery 15    │  118.13ms │                      130.72ms │  1.11x slower 
│
   > │ QQuery 16    │   68.90ms │                       72.23ms │     no change 
│
   > │ QQuery 17    │  216.46ms │                      213.50ms │     no change 
│
   > │ QQuery 18    │  319.42ms │                      322.68ms │     no change 
│
   > │ QQuery 19    │  116.85ms │                      115.95ms │     no change 
│
   > │ QQuery 20    │  135.26ms │                      123.83ms │ +1.09x faster 
│
   > │ QQuery 21    │  279.46ms │                      275.56ms │     no change 
│
   > │ QQuery 22    │   68.17ms │                       67.74ms │     no change 
│
   > 
└──────────────┴───────────┴───────────────────────────────┴───────────────┘
   > ```
   
   I believe this is expected to have no change:
   We already have `StringView -> substr() -> StringView`
   This PR just changed `StringArray -> substr() -> StringArray` to 
`StringArray -> substr() -> StringView`
   
   The benchmark runs on Parquet, which read to StringView by default.
   The TPCH test result changes due to this PR because it runs on CSV (.tbl). 
Allowing the CSV scanner to produce StringView seems like an optimization we 
could do


-- 
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-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org