Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in sql.md: fix the query to ignore nulls #1738

Merged
merged 1 commit into from
Oct 10, 2024
Merged

bug in sql.md: fix the query to ignore nulls #1738

merged 1 commit into from
Oct 10, 2024

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Oct 10, 2024

I see this bug in the last chart of https://observablehq.com/framework/sql

spurious-rect

this spurious rect corresponds to datum 34 which has mag1 = mag2 = null. Plot does not handle correctly the nulls in arrow vectors.

It's still unclear to me why it does not happen all the time, and why it happens at all.

@Fil Fil requested a review from mbostock October 10, 2024 09:22
@Fil
Copy link
Contributor Author

Fil commented Oct 10, 2024

And this is how it looks like under the duckdb-1.29 branch
broken-1 29

@Fil
Copy link
Contributor Author

Fil commented Oct 10, 2024

And sometimes when I'm just fiddling with the code block, it looks like this:
bug-0-100

even if I'm not changing the query!

@Fil
Copy link
Contributor Author

Fil commented Oct 10, 2024

This seems to be an issue with the native handling of arrow tables in @observablehq/plot@0.6.16 in fact, because the bug disappears when I use @observablehq/plot@0.6.15.

@Fil Fil closed this Oct 10, 2024
@Fil Fil deleted the fil/ignore-nulls branch October 10, 2024 10:03
@Fil Fil restored the fil/ignore-nulls branch October 10, 2024 12:35
@Fil
Copy link
Contributor Author

Fil commented Oct 10, 2024

now that I understand why it happens, maybe it's better to fix it also independently of Plot’s observablehq/plot#2194

@Fil Fil reopened this Oct 10, 2024
@Fil Fil marked this pull request as ready for review October 10, 2024 14:59
@Fil Fil merged commit 4be44b4 into main Oct 10, 2024
8 checks passed
@Fil Fil deleted the fil/ignore-nulls branch October 10, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants