Set up SEARCH_PATH only once when running an update command

[MalloD12]

Impact

• Bug fix (non-breaking change which fixes expected existing functionality)
• Enhancement/New feature (adds functionality without impacting existing logic)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Fixes #5316.

This PR provides a new DatabaseUtils method to set up SEARCH_PATH variable permanently when running any update family command. Previously when running a update-sql command on a Postgres DB we could see multiple SET SEARCH_PATH... statements in the generated SQL output. It seems that was happening because of the DatabaseUtils.initializeDatabase() call performed in the rollback() method from PostgresDatabase implementation. With this change that's not happening anymore, but SQL output will be a bit different now, since what we will see is:

ALTER DATABASE lbcat SET SEARCH_PATH TO test_schema_daniel, "$user","public";   

only once.

Things to be aware of

• This a Postgres specific change.

Things to worry about

• I guess we might need to apply the same logic in PRO (update-one-changeset) if we are happy with these changes.

Additional Context

[MalloD12]

This is still a work in progress since I need to add some tests to this PR, but I would appreciate it if you guys could start reviewing these changes in the meantime.

From a testing point of view what I have done so far is to execute update/update-sql commands using a changelog with a few changesets and having:

<changeSet author="lbuser" id="4-table" labels="testtable4" contextFilter="none" failOnError="false">
        <createTable tableName="testtable4">
            <column name="supplier_id" type="numeric(4)"/>
            <column name="supplier_name" type="TTTTVARCHAR(50)"/>
        </createTable>
    </changeSet>

as one of them. Which will cause the rollback method to be triggered.

• Run update/update-sqlcommands without proving any specific schema. Output was:

-- Create Database Lock Table
CREATE TABLE public.databasechangeloglock (ID INTEGER NOT NULL, LOCKED BOOLEAN NOT NULL, LOCKGRANTED TIMESTAMP WITHOUT TIME ZONE, LOCKEDBY VARCHAR(255), CONSTRAINT databasechangeloglock_pkey PRIMARY KEY (ID));

-- Initialize Database Lock Table
DELETE FROM public.databasechangeloglock;

INSERT INTO public.databasechangeloglock (ID, LOCKED) VALUES (1, FALSE);

-- Lock Database
UPDATE public.databasechangeloglock SET LOCKED = TRUE, LOCKEDBY = 'Daniels-MacBook-Pro.local (192.168.1.28)', LOCKGRANTED = NOW() WHERE ID = 1 AND LOCKED = FALSE;

-- Create Database Change Log Table
CREATE TABLE public.databasechangelog (ID VARCHAR(255) NOT NULL, AUTHOR VARCHAR(255) NOT NULL, FILENAME VARCHAR(255) NOT NULL, DATEEXECUTED TIMESTAMP WITHOUT TIME ZONE NOT NULL, ORDEREXECUTED INTEGER NOT NULL, EXECTYPE VARCHAR(10) NOT NULL, MD5SUM VARCHAR(35), DESCRIPTION VARCHAR(255), COMMENTS VARCHAR(255), TAG VARCHAR(255), LIQUIBASE VARCHAR(20), CONTEXTS VARCHAR(255), LABELS VARCHAR(255), DEPLOYMENT_ID VARCHAR(10));

-- *********************************************************************
-- Update Database Script
-- *********************************************************************
-- Change Log: changelog/xml/showSummaryWithLabels.xml
-- Ran at: 5/01/24, 3:54 PM
-- Against: postgres@jdbc:postgresql://localhost:5438/lbcat
-- Liquibase version: [local build]
-- *********************************************************************

ALTER DATABASE lbcat SET SEARCH_PATH TO public, "$user","public";

-- Changeset changelog/xml/showSummaryWithLabels.xml::1-table::lbuser
CREATE TABLE public.testtable1 (supplier_id numeric(4), supplier_name VARCHAR(50));

....

-- Release Database Lock
UPDATE public.databasechangeloglock SET LOCKED = FALSE, LOCKEDBY = NULL, LOCKGRANTED = NULL WHERE ID = 1;

Also update was successfully executed storing changes in the public schema, as expected.

• Run update/update-sqlcommands proving --default-schema-name=test_schema_daniel schema. Output was:

-- Create Database Lock Table
CREATE TABLE test_schema_daniel.databasechangeloglock (ID INTEGER NOT NULL, LOCKED BOOLEAN NOT NULL, LOCKGRANTED TIMESTAMP WITHOUT TIME ZONE, LOCKEDBY VARCHAR(255), CONSTRAINT databasechangeloglock_pkey PRIMARY KEY (ID));

-- Initialize Database Lock Table
DELETE FROM test_schema_daniel.databasechangeloglock;

INSERT INTO test_schema_daniel.databasechangeloglock (ID, LOCKED) VALUES (1, FALSE);

ALTER DATABASE lbcat SET SEARCH_PATH TO test_schema_daniel, "$user","public";

-- Changeset changelog/xml/showSummaryWithLabels.xml::1-table::lbuser
CREATE TABLE test_schema_daniel.testtable1 (supplier_id numeric(4), supplier_name VARCHAR(50));

-- Changeset changelog/xml/showSummaryWithLabels.xml::4-table::lbuser
CREATE TABLE test_schema_daniel.testtable4 (supplier_id numeric(4), supplier_name TTTTVARCHAR(50));

...

Process finished with exit code 0

• Also execute same test (only for Postgres) that Enzo comment here.

NOTE: just in case I would like to mention part of the provided SQL outputs have been deleted to not make comments too long because of the length of them.

[StevenMassaro]

Looks good from here

[StevenMassaro]

Maybe this should be logged at a different level than info?

[MalloD12]

Right, thinking of updating this as a warning and the other one you comment below as an error. Would that be the correct logging level for both of these messages?

Thanks,
Daniel.

[StevenMassaro]

Sounds right to me

[StevenMassaro]

Log level?

[filipelautert]

@MalloD12 did you try to execute it with an user that is not database owner? Cause I tried to create an user, give him access to a database as below and then the alter failed:

--as postgres user:
create user tst with password 'tst';
create database tst;
grant connect, create on database tst to tst;

-- reconnect as tst, and then try it:
ALTER DATABASE tst SET SEARCH_PATH TO public;

[42501] ERROR: must be owner of database tst

I need to grant all privileges on database tst to tst so it works.

I can think of some options:

1. try the alter table; if it fails fall back to the set search repetition ;
2. do not rollback connections for postgresql (is it really required) then we can have only one set search_path
3. Create (another) flag so users can decide what to do.

I'm suggesting this because requiring all privileges would be a breaking change and could create problems with companies devops pipelines.

CC @StevenMassaro