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

Migrate all NTZ in task_log back to LTZ #660

Merged
merged 4 commits into from
May 9, 2024

Conversation

joshelser
Copy link
Contributor

Description

The times in admin.materialization_status don't have timezones but are set to the current session timezone. This leads to confusion where downstream systems may interpret the time as UTC instead of the actual timezone.

This changes the fields in INTERNAL.TASK_LOG from TIMESTAMP_NTZ to TIMESTAMP_LTZ which implicitly carry through to ADMIN.MATERIALIZATION_STATUS.

To avoid weirdness of TIMESTAMP_LTZ values changes through a procedure call, this avoids passing start_time back when in FINISH_TASK. If you're curious:

begin
    create or replace table tztest(ts timestamp_ltz);
    WITH example AS PROCEDURE (ts timestamp_ltz)
      RETURNS STRING
    AS
    $$
    BEGIN
        insert into tztest select :ts;
        return '';
    end
    $$
    CALL example(current_timestamp());
    
    let rs resultset :=(select *, current_timestamp() from tztest);
    return table(rs);
end;

There is no (easy) way to correct the data in-place. We have to do this before we release the native app with the task_log publicly; else, we will have a very bad migration problem on our hands.

Checklist:

  • I have verified that my change is functionally correct (unit tests are great).
  • Queries against newly-added, user-facing objects can be expected to complete in 1's of seconds.
  • My change will not have a substantial increase the user's cost to have the app installed (did you add any new tasks?).

The times in admin.materialization_status don't have timezones but are
set to the current session timezone. This leads to confusion where
downstream systems may interpret the time as UTC instead of the actual
timezone.

To avoid weirdness of TIMESTAMP_LTZ values changes through a procedure
call, this avoids passing start_time back when in FINISH_TASK.

There is no way to correct the data in-place. We have to do this before
we release the native app publicly.
@joshelser joshelser requested a review from rymurr May 9, 2024 02:03
rymurr
rymurr previously approved these changes May 9, 2024
@joshelser joshelser requested a review from rymurr May 9, 2024 17:22
@joshelser joshelser merged commit d3ee7e0 into sundeck-io:main May 9, 2024
7 checks passed
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