You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Rich-T-kid (via GitHub)" <gi...@apache.org> on 2026/08/05 17:53:43 UTC

[I] Epic: Cost-Based Dictionary Encoding [datafusion]

Rich-T-kid opened a new issue, #24111:
URL: https://github.com/apache/datafusion/issues/24111

   ### Is your feature request related to a problem or challenge?
   
   todo
   
   ### Describe the solution you'd like
   
   todo
   
   ### Describe alternatives you've considered
   
   todo
   
   ### Additional context
   
   todo


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "asolimando (via GitHub)" <gi...@apache.org>.
asolimando commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5372759576

   Apologies for the late reply. I agree with @alamb that it's fine to start with a configuration option to let the user activate when they know it helps, and work our way towards automatic activation.
   
   We synced offline with @Rich-T-kid and indeed what will make automatic activation a little tricky, is that there is an overhead for the dictionary encoding per record batch. We don't have any statistic estimating that at the moment, and cardinality/row count isn't really a good proxy, but empirical testing will tell, it might be possible that we can still find some reasonable cost-model for this.
   
   This said, I am fully supportive of the initiative and the direction that emerged from the discussion looks good to me!
   
   


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5206283467

   Specifically, i think this one
   - https://github.com/apache/datafusion/issues/24112
   
   Would be good way to let people use dictionary more
   
   


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5404601823

   @Rich-T-kid Please tag me on all related PRs. I will be happy to help.


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5206277128

   > 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


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5404657375

   > [@Rich-T-kid](https://github.com/Rich-T-kid) Please tag me on all related PRs. I will be happy to help.
   
   thank you! currently just #24227 is in a reviewable state.


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5416573631

   > > I do think Dictionary support is important
   > 
   > Thank you [@alamb](https://github.com/alamb) and since you already like the general direction. I will work with [@Rich-T-kid](https://github.com/Rich-T-kid) to move this forward.
   
   Thank yoU @kumarUjjawal and @Rich-T-kid 
   
   One concern I have is that we don't explode the code size too much -- for example if every kernel now gets dictionary handling that is likely to generate many many copies of the code
   
   For example, something like this is happening in IN LISTs where each specialization also has a dictioanry dispatch
   - https://github.com/apache/datafusion/issues/24658
   
   In general I think the pattern  pattern for Dictionary types should be to apply the operation to the values array, and then apply via `take` and the indices. 
   
   For example to support hashing on a dictionary array, the values should be hashed and then when a key hash is needed, look up in the corresponding values hashs
   
   


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5401316069

   > > [@alamb](https://github.com/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.
   > 
   > Thanks -- I am sorry I just don't have time to review all the PRs anymore. I do think Dictionary support is important, and so am trying to help with the project but I don't want to lead you on and imply I have bandwidth to help drive / review this project in any meaningful way at the moment
   
   understandable, your advice is still very appreciated.


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5417039428

   This is a problem that came to mind but it shouldn't be an issue. This epic shouldn't be affect how existing kernels/operators interact with the dictionary data type. Aggregations now natively supporting `dict<_,v>`, so once a column is dictionary encoded it is never cast to its value type. 
   
   
   > In general I think the pattern pattern for Dictionary types should be to apply the operation to the values array, and then apply via take and the indices.
   
   > For example to support hashing on a dictionary array, the values should be hashed and then when a key hash is needed, look up in the corresponding values hashes
   
   makes sense to me, will keep in mind


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5401210613

   > @alamb, 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.
   
   Thanks -- I am sorry I just don't have time to review all the PRs anymore. I do think Dictionary support is important, and so am trying to help with the project but I don't want to lead you on and imply I have bandwidth to help drive / review this project in any meaningful way at the moment


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "kumarUjjawal (via GitHub)" <gi...@apache.org>.
kumarUjjawal commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5404594969

   > I do think Dictionary support is important
   
   Thank you @alamb and since you already like the general direction. I will work with @Rich-T-kid to move this forward.


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5207894216

   >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.
   
   this is a valid point, I think focusing on parquet for now seems like a safe bet since as you mentioned its much more efficent due to RLE encoding.
   
   >Specifically, i think this one
   
   > > https://github.com/apache/datafusion/issues/24112
   
   >Would be good way to let people use dictionary more 
   
   agreed. will work on plumbing this with arrow-rs


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

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


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


Re: [I] Epic: Cost-Based Dictionary Encoding [datafusion]

Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #24111:
URL: https://github.com/apache/datafusion/issues/24111#issuecomment-5195660049

   @alamb @kumarUjjawal @zhuqi-lucas @asolimando I would appreciate any thoughts you have on this


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

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


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