Rich-T-kid commented on issue #24111: URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5374679726
> > Together these give users manual control first (enable dictionary reads, set a cardinality threshold), then build the statistics and optimizer support needed for DataFusion to make that same decision automatically. The end goal is a physical plan optimization that casts low-cardinality string columns to dictionary arrays before aggregation when the stats justify it, without requiring any manual intervention. > > I think one challenge with this approach is that casting to dictionary will likely be just as expensive as the grouping itself so I am not sure if this will actually save much work. > > we could potentially do something like add some heuristic of when to read from parquet as DictionaryArrays (as reading directly into Dictionary from parquet can be fast when it is physically encoded that way) > > I think the first step of any such project woudl be to make sure that DataFusion can take full advantage of dictionary arrays when the user specifies them -- then we can move on to trying to automatically chose to use them more frequently @alamb, [#24227](https://github.com/apache/datafusion/pull/24227) does exactly this. The diff (+943, -19) makes the PR look much larger than it actually is: roughly 400-500 LOC is tests covering a variety of edge cases (mixed column encoding within a single file, mixed encoding across multiple files, reading non-VLD types, verifying the correct schema propagates between operators, SLT tests, etc.), and another large chunk is just propagating the flag throughout the codebase. I enabled the flag to validate my assumption that blindly enabling this isn't a good default, and the results confirm it ([clickbench](https://github.com/apache/datafusion/pull/24227#issuecomment-5365010866), [sf1](https://github.com/apache/datafusion/pull/24227#issuecomment-5364948872)). As cardinality grows, performance degrades, since we lose the benefit of the keys array being a smaller, cheaper proxy for the real values. Many of the RLE_DICTIONARY-encoded columns here are extremely high cardinality (5-15 million distinct values), which drags overall query performance down. This is exactly what #24113 is meant to solve, once #24227 ships. -- 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]
