Skip to content

Conversation

hawkfish
Copy link
Contributor

@hawkfish hawkfish commented Apr 3, 2024

We used to have somewhat strange comparison semantics for nested types
where the top level produced NULL when comparing NULLs but we used
DISTINCT semantics lower down. This was surprising, didn't match Postgres,
and wasn't stable if you pulled the contents up a level e.g., via UNNEST.

Several tests were relying on these semantics. Most of them could
be fixed by changing them to using DISTINCT comparisons, but a small number
were producing incorrect results and have been corrected.

fixes: #11292
fixes: https://github.com/duckdblabs/duckdb-internal/issues/1657

hawkfish added 2 commits April 2, 2024 15:47
We used to have somewhat strange comparison semantics for nested types
where the top level produced NULL when comparing NULLs but we used
DISTINCT semantics lower down. This was surprising, didn't match Postgres,
and wasn't stable if you pulled the contents up a level e.g., via UNNEST.

Several tests were relying on these semantics. Most of them could
be fixed by changing them to using DISTINCT comparisons, but a small number
were producing incorrect results and have been corrected.

fixes: duckdb#11292
fixes: duckdblabs/duckdb-internal#1657
@hawkfish hawkfish requested review from Maxxen, lnkuiper and Mytherin April 3, 2024 15:02
Copy link
Contributor

@lnkuiper lnkuiper left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! I just have one remark:

@github-actions github-actions bot marked this pull request as draft April 4, 2024 16:57
@hawkfish hawkfish marked this pull request as ready for review April 4, 2024 16:58
PR Feedback: Undo optional_ptr for __restrict keyword arguments.

fixes: duckdb#11292
fixes: duckdblabs/duckdb-internal#1657
@github-actions github-actions bot marked this pull request as draft April 4, 2024 17:22
@hawkfish hawkfish marked this pull request as ready for review April 4, 2024 17:26
@hawkfish hawkfish added the Needs Documentation Use for issues or PRs that require changes in the documentation label Apr 4, 2024
Copy link
Contributor

@lnkuiper lnkuiper left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! LGTM

@Mytherin Mytherin merged commit 32c1c67 into duckdb:main Apr 5, 2024
@Mytherin
Copy link
Collaborator

Mytherin commented Apr 5, 2024

Thanks!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Apr 5, 2024
Merge pull request duckdb/duckdb#11496 from hawkfish/nested-nulls
@Teggy
Copy link

Teggy commented Apr 6, 2024

Wonderful! Thank you so much, folks!

@hawkfish hawkfish deleted the nested-nulls branch April 8, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Documentation Use for issues or PRs that require changes in the documentation Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comparisons of structs with NULL fields turn out wrong
4 participants