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

Snowflake some session parameters (WEEK_START) cannot be overruled in Connection String #16114

Closed
JadAbdallah opened this issue May 17, 2021 · 5 comments · Fixed by #19375
Closed

Comments

@JadAbdallah
Copy link

JadAbdallah commented May 17, 2021

Describe the bug
Hello! I have reason to believe the Metabase UI is doing some default timezone conversions causing grouping by date inconsistenties when using SQL queries. The same select "timestamp" query in Snowflake and in Metabase is returning different results.

To Reproduce
Steps to reproduce the behavior:

  1. Run select date_trunc('week', current_timestamp)::string in data warehouse (Snowflake)
  2. un select date_trunc('week', current_timestamp)::string in Metabase SQL query

Expected behavior
I would expect the same timestamp to be returned.

Screenshots
Localization set up:
Screen Shot 2021-05-17 at 2 10 25 PM

Information about your Metabase Installation:

You can get this information by going to Admin -> Troubleshooting.

  • Your databases: (e.x. MySQL, Postgres, MongoDB, …) Snowflake
  • Metabase version: (e.x. 0.19.3) v0.39.0.1

Severity
How severe an issue is this bug to you? Is this annoying, blocking some users, blocking an upgrade or blocking your usage of Metabase entirely?
Annoyance

Additional context
This is causing discrepencies in data points when doing groupings by week. Warehouse will default to week starting on Monday, Metabase will start on Sunday. The two should be identical.

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

@flamber
Copy link
Contributor

flamber commented May 17, 2021

Hi @JadAbdallah
Post "Diagnostic Info" from Admin > Troubleshooting.
And post the output from the queries and what you are expecting, when you run the queries.

Metabase applies applies both week_start and timezone for Snowflake (and most other drivers too):
https://github.com/metabase/metabase/blob/master/modules/drivers/snowflake/src/metabase/driver/snowflake.clj#L44-L74
Some of this is legacy, and some is workarounds until we have a more comprehensive timezone system in place.

Sounds like a duplicate of #8804

@JadAbdallah
Copy link
Author

JadAbdallah commented May 19, 2021

Hi @flamber
Thanks for getting back so fast!

Here is the Diagnostic Info:
{ "browser-info": { "language": "en-US", "platform": "MacIntel", "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4430.85 Safari/537.36", "vendor": "Google Inc." }, "system-info": { "file.encoding": "UTF-8", "java.runtime.name": "OpenJDK Runtime Environment", "java.runtime.version": "11.0.10+9", "java.vendor": "AdoptOpenJDK", "java.vendor.url": "https://adoptopenjdk.net/", "java.version": "11.0.10", "java.vm.name": "OpenJDK 64-Bit Server VM", "java.vm.version": "11.0.10+9", "os.name": "Linux", "os.version": "5.4.95-42.163.amzn2.x86_64", "user.language": "en", "user.timezone": "UTC" }, "metabase-info": { "databases": [ "snowflake" ], "hosting-env": "unknown", "application-database": "postgres", "application-database-details": { "database": { "name": "PostgreSQL", "version": "10.15" }, "jdbc-driver": { "name": "PostgreSQL JDBC Driver", "version": "42.2.18" } }, "run-mode": "prod", "version": { "date": "2021-04-20", "tag": "v0.39.0.1", "branch": "release-x.39.x", "hash": "47bb5f2" }, "settings": { "report-timezone": null } } }

And here's the output from the queries:
In Snowflake: select date_trunc('week', current_timestamp)::string -- output is 2021-05-17 00:00:00.000 Z
In Metabase: select date_trunc('week', current_timestamp)::string -- output is 2021-05-16 00:00:00.000 Z

I'm seeing that the report-timezone setting is set to null. Should this be set to the our local timezone?

@flamber
Copy link
Contributor

flamber commented May 29, 2021

@JadAbdallah I'm having difficulties understanding if this is a problem, when you use GUI instead of SQL in Metabase.
As the "First day of week" settings says:

This will affect things like grouping by week or filtering in GUI queries. It won't affect SQL queries.

But I'm leaving this issue open, since it seems like we're overriding any session parameters defined in Admin > Databases > (db) > Connection String, which would allow you to add this: WEEK_START=1
We have similar problem with Postgres a few months back, so I guess this issue might be all databases #13797


Workaround - note that when this issue is fixed, then you might need to adjust all your SQL again (removing all the dateadd)

Example with Monday as start of week:

select date_trunc('week', current_timestamp)::string, dateadd(day, 1, date_trunc("week", dateadd(day, -1, current_timestamp)))::string

So if you are trying to do something like this in SQL:

date_trunc('week', CAST("dataset"."schema"."table"."column" AS timestamp))

Then it should be modified to this (in case of Monday):

dateadd(day, 1, date_trunc('week', dateadd(day, -1, CAST("dataset"."schema"."table"."column" AS timestamp))))

@fernandobrito
Copy link
Contributor

Hello. I ended up here after having some internal users reporting that their revenue KPIs (coming from Snowflake data) aggregated by week are not matching when they compare the Metabase numbers against the same KPIs on Tableau or queried directly from the Snowflake UI.

As mentioned by flamber, Metabase is forcing WEEK_START=7 on Snowflake, even if you try to change it from the connection string.

What are possible ways of fixing it in Metabase?

  1. Completely remove the WEEK_START=7 from the JDBC connection in Metabase.
    Snowflake would use whatever they have set as a default on Snowflake, which seems reasonable. The downside is that existing Metabase users would be impacted.

  2. Keep WEEK_START=7 on the JDBC connection parameters, but only as a fallback if the user does not set their own.
    Keeps backwards compatibility but let users override it.

  3. Use WEEK_START=? where ? is whatever the user chose on the Metabase localization settings.
    A bit confusing, since on the Metabase UI it states that

This will affect things like grouping by week or filtering in GUI queries. It won't affect SQL queries.

For our Metabase users that use Snowflake's week(), we can just ask them to use weekiso() as a temporary fix, since in our particular case our default on Snowflake is to follow the ISO semantics. However, for users using date_trunc('week', ...), like the original poster, it seems that there's no temporary workaround.

@flamber flamber changed the title Timestamp conversion issue Snowflake some session parameters (WEEK_START) cannot be overruled in Connection String Sep 4, 2021
@UserArn00b
Copy link

Hello, the issue I've created has been closed but I think I found another issue related to this topic.
When I drill down into data from a chart created on a weekly basis, the dates in the tables are not matching the week I drill down into.
In the example, I want to display week 40 data, but the 29th of september is actually in week 39.
It is really annoying for end users :(
drill_down_issue

@jeff303 jeff303 linked a pull request Dec 15, 2021 that will close this issue
@flamber flamber added this to the 0.42 milestone Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants