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

The chart draws Zero value for NULL instead of hiding the value #4122

Open
anki-code opened this issue Jan 11, 2017 · 18 comments

Comments

@anki-code
Copy link

commented Jan 11, 2017

image

(Clarification: SELECT NULL::int return NULL in Vertica. In case if you think that NULL::int is 0 value)

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@tlrobinson

This comment has been minimized.

Copy link
Member

commented Jan 11, 2017

I believe this isn't a regression, Metabase has always treated NULL as 0.

It does treat actual missing values (when it can detect the interval) differently. We have a "missing values" setting to change the behavior of that, but it looks like it doesn't apply to in this case.

@salsakran What's the expected behavior here?

@salsakran

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

For time series, we treat a null value as a 0.

The reason for this is that if you do a group by, empty buckets are returned as nulls and not 0.

I see linear interpolation on timeseries with nulls being the exceptional case, and something that imo is incorrect when rendering timeseries with aggregate metrics as the value.

@salsakran

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

i.e. this is absolutely expected behavior.

@salsakran

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

closing unless we want to add interpolation controls on normal timeseries.

@tlrobinson

This comment has been minimized.

Copy link
Member

commented Jan 11, 2017

@salsakran

The reason for this is that if you do a group by, empty buckets are returned as nulls and not 0.

The backend doesn't return a row at all for empty buckets. I believe we have/had a ticket open for a long time for that issue but we might have closed it when we added the Replace missing values with setting on the frontend, though the default is LInear Interpolated (I believe we didn't want to break existing behavior)

This is the default:

screenshot 2017-01-11 12 06 07

This is what I think you're expecting the default is:

screenshot 2017-01-11 12 09 07

I think ideally we should filter NULL rows (so the behavior is more consistent and the missing value setting works for NULLS) and default to Zero in more (all?) cases.

@salsakran

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

Ahhh, I see. I was probably too quick to the draw.

@salsakran salsakran reopened this Jan 11, 2017

@salsakran

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

can anyone think of any situations where there's a semantic difference between a null and a missing value?

@tlrobinson

This comment has been minimized.

Copy link
Member

commented Jan 11, 2017

The issue I was talking about was #896

I wonder how many cards we would break vs. make "more correct" if we changed the default from "Linear Interpolated" to "Zero"?

@camsaul

This comment has been minimized.

Copy link
Member

commented Jan 11, 2017

I would think zero should be the default e.g. if we have a query that returns number of sales per month you don't want to interpolate months where you had no sales

@salsakran

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

Ugh.

Regardless, I think I'm on board with nulls being treated as missing values for the sake of the interpolation setting you mentioned, @tlrobinson

@anki-code

This comment has been minimized.

Copy link
Author

commented Jan 11, 2017

When you write WHERE val != 1 on sql it really returns rows where val != 1 AND val is not null. It's standard behavior - NULL is "no value".

@camsaul

This comment has been minimized.

Copy link
Member

commented Jan 11, 2017

@anki-code we know 😉

@anki-code

This comment has been minimized.

Copy link
Author

commented Feb 27, 2017

It's really annoying when NULL values are propagating to all area chart as Zero.

On chart below you can see the release of new version of app. But the "new version area" hasn't started in the release date point. It has NULL value from first date.

image

@tlrobinson tlrobinson self-assigned this Feb 27, 2017

@tlrobinson

This comment has been minimized.

Copy link
Member

commented Feb 27, 2017

@anki-code I'll take a look at this.

@salsakran I think we decided nulls should basically be treated the same as "missing" values?

@anki-code

This comment has been minimized.

Copy link
Author

commented Feb 27, 2017

Could be a solution is to add an option here:

image

@rthouvenin

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2017

Any chance that this issue is looked at soon? If not enough support from the community, can someone point me to the relevant places in the code so I can give a try?

@anki-code

This comment has been minimized.

Copy link
Author

commented Dec 7, 2017

Still actual in 0.27.1

@jornh

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2017

@rthouvenin looks like the interpolation setting mentioned above in #4122 (comment) is applied in https://github.com/metabase/metabase/blob/v0.27.1/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js#L300 so that’s one starting point if you are still up for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.