DATABASECHANGELOGLOCK column LOCKGRANTED does not use timezone

[Marcono1234]

Environment

Liquibase Version:
Latest (currently 48e1056)?

Liquibase Integration & Version: <Pick one: CLI, maven, gradle, spring boot, servlet, etc.>
Programmatic (?)

Liquibase Extension(s) & Version:

Database Vendor & Version:
PostgreSQL 13.3
(Docker container running on WSL)

Operating System Type & Version:
Windows 10

Description

It appears the DATABASECHANGELOGLOCK table has a LOCKGRANTED column without timezone information.
Therefore when one client acquires the lock, and afterwards a second client with a different timezone queries the lock, for example using new Liquibase(...).listLocks(), a wrong time will be reported by DatabaseChangeLogLock.getLockGranted().

The reason for this is probably that here datetime is used, which is apparently mapped by DateTimeType to TIMESTAMP (without timezone information).

In case you decide to change this, the documentation would have to be adjusted as well.

Maybe this affects columns of other tables as well, probably column DATEEXECUTED of table DATABASECHANGELOG.

Steps To Reproduce

1. Start a PostgreSQL database
2. With one client using timezone UTC+2 call

   LockServiceFactory.getInstance().getLockService(...).acquireLock();

3. On a different client with UTC-0 call

   new Liquibase(...).listLocks()[0].getLockGranted()

   ❌ The reported datetime is 2 hours in the future

Actual Behavior

LOCKGRANTED has no timezone information.

Expected/Desired Behavior

LOCKGRANTED should store timezone information.

┆Issue is synchronized with this Jira Story by Unito

[molivasdat]

Hi @Marcono1234 Thanks for creating this issue. This may not be a simple change for existing users. We will add this to the list of issues that we are processing.

[sync-by-unito]

➤ Wesley Willard commented:

Given that changing the data type of the LOCKGRANTED column in the DATABASECHANGELOGLOCK table is not trivial, our best bet is to display the database server’s timezone information during listLocks. My initial research doesn’t show a database-agnostic way to do that, so how we do that looks like it will need to be different for each one.

[sync-by-unito]

➤ Surya Aki commented:

Please get the build from GH Actions
cc Wesley Willard Erzsebet Carmean Jack Odzhubeiskyi obovsunivskyi

[sync-by-unito]

➤ Erzsebet Carmean commented:

Liquibase Core: LB2126/794/648e2a/202111-16 18:55+0000, b794

List-locks is not outputting the lockgranted time in the timezone of the database. I validated this by starting a Postgres docker instance with the timezone Africa/Lusaka.

Docker Container Started with TimeZonedocker run --name pg_TZ_Africa -p5435:5432 -e POSTGRES_PASSWORD=password -e TZ=Africa/Lusaka postgres:12.6

I verify the timezone by running this query using psql:

SELECT * FROM pg_timezone_names WHERE name = current_setting('TIMEZONE')

The query result is:

 name      | abbrev | utc_offset | is_dst

---------------+--------+------------+--------
Africa/Lusaka | CAT | 02:00:00 | f
(1 row)Then, I create a table in my postgres database.

CREATE TABLE accounts (
name VARCHAR(100) NOT NULL,
balance DEC(15,2) NOT NULL
);To force the liquibase lock to stay for as long as the timeout is set, I begin a Postgres transaction to insert data into accounts. Be sure to disable the autocommit feature of whatever SQL workbench you are using!

BEGIN;

INSERT INTO accounts(name,balance)
VALUES('Abernathy',50000);Start a script that runs list-locks in a loop:

#!/bin/sh

while (true)
do
../liquibase listLocks
doneExecute a liquibase update to deploy an insert statement to the table that is locked by the in-progress transaction to insert into accounts.

--liquibase formatted sql

--changeset liquibase:lb2126
INSERT INTO accounts(name,balance)
VALUES('Alice',10000);The list-locks should show the time of the lock granted as the CURRENT_TIME on the Postgres instance.

postgres=# SELECT CURRENT_TIME;
current_time

23:51:20.248346+02
(1 row)Instead, list-locks is showing me the time exactly as it was input to the DATABASECHANGELOGLOCK table by my Liquibase client (which is in my local machine time, CST (UTC/GMT-6)).

Database change log locks for postgres@jdbc:postgresql://localhost:5435/postgres

• LENOVO (172.21.224.1) at Dec 2, 2021 3:54:40 PMQuerying the DATABASECHANGELOGLOCK table directly produces:

postgres=# select * from DATABASECHANGELOGLOCK;
id | locked | lockgranted | lockedby
----+--------+----------------------------+-----------------------
1 | t | 2021-12-02 15:54:40.853956 | LENOVO (172.21.224.1)
(1 row)Any suggestions as to what I’m doing wrong, Wesley Willard ?

[Marcono1234]

@nvoxland, the linked pull request will only fix this issue for newly set up databases, right? That is fine for me as well, but I am just curious.

I also mentioned the DATEEXECUTED column in the description, but it was easy to overlook:

Maybe this affects columns of other tables as well, probably column DATEEXECUTED of table DATABASECHANGELOG.

Will this be addressed as well or should I open a new issue for that?

[nvoxland]

I'll actually take the PR link off this issue because that PR is really just a work-around and not a real fix for this issue.

Long-term we maybe should have the column type include the timezone when possible, but that will have to fall into the larger refactoring of the databasechangeloglock table we have planned.

The #2217 only fixes up the locking SQL so it uses a NOW() function vs. a datetime string from the client so that at least we have a consistent time in the database regardless of how the client is configured.

There is a similar issue with the databasechangelog table too, but the refactoring of that table will be after the databasechangeloglock logic, so it's probably worth holding off on creating the similar ticket for databasechangelog until we know for sure what we're doing with this ticket.

[Marcono1234]

Thanks a lot for the additional information. I am bit confused now that this issue was closed given that you said the PR is "not a real fix", but I assume this issue was closed intentionally?

[kataggart]

sorry @Marcono1234 you are right.. I closed this accidentally. Thanks for calling this out. I will re-open and but it back into the backlog so we still have this background when we start to look at more holistic improvements to databasechangeloglock table.