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

Support for DateTime([timezone]) time fields consistently #625

Closed
oplehto opened this issue Sep 12, 2024 · 3 comments · Fixed by #639
Closed

Support for DateTime([timezone]) time fields consistently #625

oplehto opened this issue Sep 12, 2024 · 3 comments · Fixed by #639
Assignees
Milestone

Comments

@oplehto
Copy link

oplehto commented Sep 12, 2024

It seems like in certain cases DateTime values with a timezone i.e. (DateTime('America/Chicago')) are not considered valid. For basic timeseries and tables they work but in some cases they fail.

Examples that I found:

  • Timestamp column for the logs panel
  • Lookup of default timestamp values. Fix here should be as easy as adjusting this query with something likesplitByChar('(',type)[1] AS type?. At the same time it could be optimized with a GROUP BY type, name to return only distinct values
    const TABLES_QUERY = "SELECT name,database,table,type FROM system.columns WHERE type LIKE 'Date32%' OR type LIKE 'DateTime64%' OR type = 'UInt32' OR match(type,'^DateTime$|^DateTime\\\\([^)]+\\\\)$') OR match(type,'^Date$|^Date\\([^)]+\\)$') ORDER BY type,name FORMAT JSON";

There are probably other situations as well. Seems like some type matching is happening where only DateTime and DateTime64(N) are considered timestamps.

While doing this it should be ensured that the timezone-enabled timestamps work correctly and do not get skewed (i.e. timezone shift done twice accidentally so a GMT+4 becomes GMT+8)

@Slach Slach added this to the 3.2.4 milestone Sep 12, 2024
@Slach
Copy link
Collaborator

Slach commented Oct 3, 2024

dropdown should be fixed in 6ea625b

@Slach
Copy link
Collaborator

Slach commented Oct 3, 2024

@oplehto i tried to reproduce
look #638

I created table which contains DateTime('Europe/Moscow') server in UTC and create dashboard
which use this field as DateTime

I don't see any issues

WHERE $timeFilter works correctly
used time range always in timestamp as int and properly converted from browser timezone to field timezone

Could you provide more concrete examples for your issue?

@Slach Slach closed this as completed in 8c020fa Oct 6, 2024
@Slach Slach reopened this Oct 6, 2024
@Slach Slach modified the milestones: 3.2.4, 3.2.5 Oct 6, 2024
@oplehto
Copy link
Author

oplehto commented Oct 7, 2024

An example here would be the timestamp column for the logs panel (in the logs mode) as I mentioned in the issue. I assume you are testing with a Graph or Table panel?

I think it's due to the very specific matching done here for the column type:

case 'Date':
case 'DateTime':
case 'DateTime64':
case 'DateTime64(3)':
case 'DateTime64(6)':
case 'Nullable(Date)':
case 'Nullable(DateTime)':
case 'Nullable(DateTime64)':
case 'Nullable(DateTime64(3))':
case 'Nullable(DateTime64(6))':
return FieldType.time;

There may be issues with other panel types as well. I didn't look. Hence this request to review that the timestamp with timezone support is comprehensive meaning all the panel types support it.

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