Skip to content

ARROW-10163: [Rust] [DataFusion] Add DictionaryArray coercion support to physical plans - #8359

Closed
alamb wants to merge 3 commits into
apache:masterfrom
alamb:alamb/ARROW-10159-dictionary-array-coercion
Closed

ARROW-10163: [Rust] [DataFusion] Add DictionaryArray coercion support to physical plans#8359
alamb wants to merge 3 commits into
apache:masterfrom
alamb:alamb/ARROW-10159-dictionary-array-coercion

Conversation

@alamb

@alambalamb commented Oct 6, 2020

Copy link
Copy Markdown
Contributor

NOTE: this builds on #8346 so leaving as a draft until those are merged

This PR adds basic physical expression / casting support to DataFusion. Right now, it will cause all DictionaryArray data to be unpacked into a StringArray for operations.

Ideally, DataFusion would support direct comparison / operation on DictionaryArrays, however, there isn't the necessary support in the arrow kernels yet to do so (e.g. there is no Dictionary equality comparison kernel I could find https://github.com/apache/arrow/blob/master/rust/arrow/src/compute/kernels/comparison.rs

However, this PR gets the basic queries running and I hope to contribute further optimizations as time allows and our project needs dictate.

@github-actions

Copy link
Copy Markdown

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I don't understand why order_coercion and eq_coercion are different (eq_coercion does not include string_coercion).

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in #8340, I don't think partially redundant and incomplete checks for casting support in DataFusion adds much over the actual arrow casting, so I have removed the plan time checks

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR, I just renamed some of these arguments to match the comment description

Comment threadrust/datafusion/tests/sql.rs Outdated

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the (simple) end to end test for DictionaryArray support

@alamb
alambforce-pushed the alamb/ARROW-10159-dictionary-array-coercion branch from 7dc986e to ecd4b4eCompareOctober 6, 2020 12:40
@alamb
alambforce-pushed the alamb/ARROW-10159-dictionary-array-coercion branch from ecd4b4e to e707c21CompareOctober 7, 2020 18:26
@alambalamb closed this Oct 8, 2020
@alamb
alamb deleted the alamb/ARROW-10159-dictionary-array-coercion branch October 8, 2020 14:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@alamb