joe-clickhouse commented on PR #43960: URL: https://github.com/apache/superset/pull/43960#issuecomment-5590793148
From the ClickHouse and clickhouse-connect side, it does look like this change correctly preserves explicit UTC offsets. `DateTime64(3, 'Asia/Tehran')` now returns a timezone-aware datetime, and its ISO string is `2026-01-15T12:30:00.123000+03:30` which is 09:00 UTC. The old code replaces the offset with `Z`, which changes the instant. I also tested Superset 4.1.1 and 6.1.0 against ClickHouse 26.6 with clickhouse-connect 1.8.0. Every chart data mode I tried returned this column as an epoch number, and `normalizeTimestamp` only handles strings, so this change appears to never even touch those values. SQL Lab returned the ISO string with the offset, but its results grid does not run it through `normalizeTimestamp` either. I then had some agents do some digging and they think that the reporter's display change comes from #37979, which corrected the backend epoch conversion. So for this Tehran value, 4.1.1 returned the epoch for `12:30Z` and 6.1.0 returns the epoch for `09:00Z`. The old value happened to read as the column-local time under the default UTC formatter, but it was the wrong instant. This PR makes the same correction for frontend strings, which is right, but I don't think it'll change what the reporter's chart shows. That request needs a way to choose a display timezone. Master already has a dataset timezone setting from #37014, but it currently raises on timezone-aware columns like this one. (Probably worth filing separately). All that said, it might not be correct to say `Fixes #43927` here instead just yet. For regression coverage here though, I would add fractional seconds and assert the parsed epoch. A DST fall-back pair with identical wall times but `-04:00` and `-05:00` offsets would also help becuase in testing I saw that the old normalizer collapsed those two instants into one but this PR keeps them an hour apart. Hope that helps! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
