Skip to content

fix: datatype_is_logically_equal for dictionaries#20153

Open
dd-annarose wants to merge 3 commits intoapache:mainfrom
dd-annarose:annarose/dict-coercion
Open

fix: datatype_is_logically_equal for dictionaries#20153
dd-annarose wants to merge 3 commits intoapache:mainfrom
dd-annarose:annarose/dict-coercion

Conversation

@dd-annarose
Copy link

Which issue does this PR close?

When checking logical equivalence with Dictionary<_, Utf8> and Utf8View, the response was false which is not what we expect (logical equivalence should be a transitive property).

What changes are included in this PR?

This PR introduces a test and a fix. The test fails without the fix. The fix is simply calling datatype_is_logically_equal again on the v1 and othertype when called with Dictionary<K1, V1> and othertype.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions github-actions bot added the common Related to common crate label Feb 4, 2026
Copy link
Member

@notfilippo notfilippo left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Thanks :)

Copy link
Contributor

@gabotechs gabotechs left a comment

Choose a reason for hiding this comment

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

Looks good! thanks @dd-annarose

@dd-annarose dd-annarose force-pushed the annarose/dict-coercion branch from df92e66 to 321b285 Compare February 4, 2026 16:22
@dd-annarose dd-annarose force-pushed the annarose/dict-coercion branch from 1a0c600 to 09c29d7 Compare February 4, 2026 23:11
@dd-annarose
Copy link
Author

does anyone know why the [Rust / linux build test (pull_request)](https://github.com/apache/datafusion/actions/runs/21708868623/job/62609567530?pr=20153) check is stuck? it's been like this since the first version of the PR :sad: @Jefffrey maybe?

@dd-annarose dd-annarose closed this Feb 5, 2026
@dd-annarose dd-annarose reopened this Feb 5, 2026
@dd-annarose
Copy link
Author

(trying to retrigger something sorry for the noise everyone)

@Jefffrey
Copy link
Contributor

Jefffrey commented Feb 5, 2026

No idea; maybe we missed our daily prayers to the GitHub Actions gods? Tried rerunning that specific job to no avail 🙁

@Jefffrey
Copy link
Contributor

Jefffrey commented Feb 5, 2026

Seeing if merging up from main might help 🤔

@Jefffrey
Copy link
Contributor

Jefffrey commented Feb 5, 2026

Seems to be having same problem; looks like it perhaps is running but we can't get the logs somehow? 🤔

@Jefffrey
Copy link
Contributor

Jefffrey commented Feb 5, 2026

We did recently change the runner to an AWS runner:

I guess it's related to that if it's only this job being blocked; cc @blaginin

@blaginin
Copy link
Collaborator

blaginin commented Feb 5, 2026

lets see if it'll get picked up 👀

@blaginin
Copy link
Collaborator

blaginin commented Feb 5, 2026

seems like an issue on the asf infra side, pinged the team (cc @gmcdonald), let me know if you need to merge this PR urgently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants