[11.5.362 Pro] - Executing non-trivial scripts on Postgres

[11.5.362 Pro] - Executing non-trivial scripts on Postgres

A have a moderately not-small collection of database deployment scripts for deploying a Postgres database. Alas, one of our clients does not allow the use of 3rd party database deployment solutions, hence if I don't want to manually execute dozens of script files (hundreds in the future) one by one, I have to merge them into a large, bulky, and unwieldy bunch of concatenated characters and run that on their server.

Now, on Postgres that tends to cause problems. Whenever the execution reaches the part of the script that has an anonymous block, I'm presented with a short error message, and the execution aborts:

	Quote:
	SQL State: 42601, ERROR: cannot insert multiple commands into a prepared statement...

Currently, I'm running those separately, so I'm out of the frying pan but into the fire, because that means that instead of having one large, bulky, and unwieldy bunch of concatenated characters I have several, slightly smaller, but still large, bulky, and unwieldy bunch of concatenated characters that still have to be executed one by one.

Is there a more sophisticated/convenient fix/workaround for that?

Do you get the same error when executing the script with psql command line?

Are there COMMIT; statements within the script to break it into smaller transactions?

No, there aren't any COMMITs in the script (that I'm aware of). I cannot test it from psql command line as I don't have one at hand, but when running it from the PgAdmin the client provided as an alternative method of access, there were no errors.

Could you please share code of the anonymous block triggering SQL State: 42601 error, or a similar code sample demonstrating what is being executed there?

Sure, here goes one of the culprits. I posted it in its standalone form. I haven't tried but I think adding anything to it might make the error come to life.

Code:

-- Version: 0.22.101
-- hkir_gw_v0.22.101__mod.column defaults from clock ts to utc ts.default.sql

-- #region deployment script start hkir_gw_r__000147__mod.column defaults from clock ts to utc ts.default.sql -------------------------------------------------------------------------------------------------------
-- ----------------------------------------------------------------------------------------------------------------------------------------------------

-- #region == == the essence starts here == == == == == == == == == == == == == == == == == == == == == == == == == == == == ==

DO
$$
DECLARE
"lv_change_def_stmt" text;
"lv_rec" record;
BEGIN
FOR "lv_rec" IN (
SELECT DISTINCT
pa."attrelid"::regclass AS "full_name"
,pa."attname" AS "column_name"
,(parse_ident(pa."attrelid"::regclass::text))[1] AS "schema_name"
,(parse_ident(pa."attrelid"::regclass::text))[1] AS "table_name"
,concat('ALTER TABLE ', pa."attrelid"::regclass, ' ALTER COLUMN ', quote_ident(pa."attname"), ' SET DEFAULT (now() at time zone ''utc'');') AS "change_stmt"
,pa."atthasdef"
,pg_get_expr(pd."adbin", pd."adrelid")
,pd."adnum"
,pa."attnum"
FROM "pg_catalog"."pg_attribute" AS pa
INNER JOIN "pg_catalog"."pg_class" AS pc ON pc."oid" = pa."attrelid"
LEFT OUTER JOIN "pg_catalog"."pg_attrdef" AS pd ON pd."adrelid" = pa."attrelid" AND pd."adnum" = pa."attnum"
WHERE 1 = 1
AND pa."attname" IN ('created', 'modified')
AND (parse_ident(pa."attrelid"::regclass::text))[1] NOT IN ('tada', 'information_schema')
AND pg_get_expr(pd."adbin", pd."adrelid") = 'clock_timestamp()'
AND NOT pa."attisdropped"
) LOOP
"lv_change_def_stmt" := "lv_rec"."change_stmt";
RAISE NOTICE E'Statement to execute: %', "lv_change_def_stmt";
EXECUTE "lv_change_def_stmt";
END LOOP;
END
$$
;

-- #endregion == the essence ends here = == == == == == == == == == == == == == == == == == == == == == == == == == == == == ==

-- #endregion deployment script ends hkir_gw_r__000147__mod.column defaults from clock ts to utc ts.default.sql -----------------------------------------------------------------------------------------------------

I'm looking at the code and thinking that the error might be actually valid. May I ask for a couple of minor changes and trying them?

1. Add filter on the object class, in the JOIN clause add the following condition to ensure that ALTER is executed for tables only
AND pc.relkind = 'r'

2. Wrap EXECUTE in try...catch

	Code:
	BEGIN EXECUTE ... EXCEPTION WHEN OTHERS THEN RAISE WARNING ... report that as a warning and continue END;

Aside from that I'd recommend removing all double quotes surrounding variable names, this is uncommon. Like in any other language in Pl/pgSQL variable names should follow regular naming conventions. I actually tested and removed all of them from your code sample and it did run just fine on my end. Here is the updated version with my modifications proposed above

Code:

-- Version: 0.22.101
-- hkir_gw_v0.22.101__mod.column defaults from clock ts to utc ts.default.sql

-- #region deployment script start hkir_gw_r__000147__mod.column defaults from clock ts to utc ts.default.sql -------------------------------------------------------------------------------------------------------
-- ----------------------------------------------------------------------------------------------------------------------------------------------------

-- #region == == the essence starts here == == == == == == == == == == == == == == == == == == == == == == == == == == == == ==

DO
$$
DECLARE
lv_rec record;
BEGIN
FOR lv_rec IN (
SELECT DISTINCT
pa.attrelid::regclass AS full_name
,pa.attname AS column_name
,(parse_ident(pa.attrelid::regclass::text))[1] AS schema_name
,(parse_ident(pa.attrelid::regclass::text))[1] AS table_name
,concat('ALTER TABLE ', pa.attrelid::regclass, ' ALTER COLUMN ', quote_ident(pa.attname), ' SET DEFAULT (now() at time zone ''utc'');') AS change_stmt
,pa.atthasdef
,pg_get_expr(pd.adbin, pd.adrelid)
,pd.adnum
,pa.attnum
FROM pg_catalog.pg_attribute AS pa
INNER JOIN pg_catalog.pg_class AS pc ON pc.oid = pa.attrelid AND pc.relkind = 'r' -- tables only
LEFT OUTER JOIN pg_catalog.pg_attrdef AS pd ON pd.adrelid = pa.attrelid AND pd.adnum = pa.attnum
WHERE 1 = 1
AND pa.attname IN ('created', 'modified')
AND (parse_ident(pa.attrelid::regclass::text))[1] NOT IN ('tada', 'information_schema')
AND pg_get_expr(pd.adbin, pd.adrelid) = 'clock_timestamp()'
AND NOT pa.attisdropped
) LOOP
RAISE NOTICE E'Statement to execute: %', lv_rec.change_stmt;
-- EXECUTE lv_change_def_stmt;
   BEGIN
      EXECUTE lv_rec.change_stmt;
   EXCEPTION
      WHEN OTHERS THEN
         RAISE WARNING '==== Failed to execute % ==== ', lv_rec.change_stmt;
   END;
END LOOP;
END
$$
;

-- #endregion == the essence ends here = == == == == == == == == == == == == == == == == == == == == == == == == == == == == ==

-- #endregion deployment script ends hkir_gw_r__000147__mod.column defaults from clock ts to utc ts.default.sql -----------------------------------------------------------------------------------------------------

Just in case, here is a simplified version of the same code

Code:

-- Version: 0.22.101
-- hkir_gw_v0.22.101__mod.column defaults from clock ts to utc ts.default.sql

-- #region deployment script start hkir_gw_r__000147__mod.column defaults from clock ts to utc ts.default.sql -------------------------------------------------------------------------------------------------------
-- ----------------------------------------------------------------------------------------------------------------------------------------------------

-- #region == == the essence starts here == == == == == == == == == == == == == == == == == == == == == == == == == == == == ==

DO
$$
DECLARE
lv_rec record;
BEGIN
FOR lv_rec IN (
SELECT DISTINCT
'ALTER TABLE ' || pa.attrelid::regclass || ' ALTER COLUMN ' || quote_ident(pa.attname) || ' SET DEFAULT (now() at time zone ''utc'')' AS change_stmt
FROM pg_catalog.pg_attribute AS pa
JOIN pg_catalog.pg_class AS pc ON pc.oid = pa.attrelid AND pc.relkind = 'r' -- tables only
JOIN pg_catalog.pg_attrdef AS pd ON pd.adrelid = pa.attrelid AND pd.adnum = pa.attnum AND pd.adsrc LIKE '%clock_timestamp%'
JOIN pg_catalog.pg_namespace AS pn ON pn.oid = pc.relnamespace AND pn.nspname NOT IN ('tada', 'information_schema')
WHERE pa.attname IN ('created', 'modified')
AND NOT pa.attisdropped
) LOOP
BEGIN
RAISE NOTICE 'Statement to execute: %', lv_rec.change_stmt;
EXECUTE lv_rec.change_stmt;
EXCEPTION
WHEN OTHERS THEN
RAISE WARNING '==== Failed to execute % ==== ', lv_rec.change_stmt;
END;
END LOOP;
END
$$
;

-- #endregion == the essence ends here = == == == == == == == == == == == == == == == == == == == == == == == == == == == == ==

-- #endregion deployment script ends hkir_gw_r__000147__mod.column defaults from clock ts to utc ts.default.sql -----------------------------------------------------------------------------------------------------

	SysOp wrote:
	I'm looking at the code and thinking that the error might be actually valid.

It cannot be. If it were, it wouldn't run in other editors, but it does. It might be that this cannot work the way SA executes (or makes the server execute) the code, but even if it is so, the counterexample shows that there are other, superior ways. The question is whether SA can be convinced to follow those instead.

As for the suggested changes,

	SysOp wrote:
	1. Add filter on the object class, in the JOIN clause add the following condition to ensure that ALTER is executed for tables only AND pc.relkind = 'r'

this one and the simplified code are good ones, though it does not change the fact that the code still fails. The surplus columns are left in the code for debugging purposes, it will be easier to remove them all at once when everything works (near) perfectly than to have to add them every time execution runs into trouble or does something unexpected.

SysOp wrote:

2. Wrap EXECUTE in try...catch

	Code:
	BEGIN EXECUTE ... EXCEPTION WHEN OTHERS THEN RAISE WARNING ... report that as a warning and continue END;

This is a no-go. The deployment codes are run by a dedicated deployer application, that handles the errors, and those errors should not be shrouded from the app. Also, this particular code is there to fix some earlier design mistakes and shall abort as soon as it turns out it's flawed and must not cause a greater mess than it is already there :)

	SysOp wrote:
	removing all double quotes surrounding variable names, this is uncommon. Like in any other language in Pl/pgSQL variable names should follow regular naming conventions.

The delimiters are going to stay. Auto-adding delimiters is one of the best features SA has. Hardly ever was there a case where everything (or rather anything at all) played by the rules. The same applies to naming conventions. It only takes one cheating player. And yes, you can reprimand. It won't help when the damage is done. It's one thing to establish some conventions and rules and it's a completely different game when enforcing those rules is resource-intensive (especially when it wastes resources you don't really have). Besides, not having delimiter surrounding names is a major PITA when it comes to refactoring the deployment code base, possibly of some legacy code that is DOA in the first place. As soon as you have names that partially match other names and you don't have them delimited, you're in deep sh... trouble. The last two times I had to do refactoring, the process literally took weeks (instead of a few hours) because those little f*ckers were missing from the code. As SA adds them automatically, you don't have to type anything, practically it has no cost. All the dicking could have been avoided simply by not disabling the auto-add delimiter feature. Since then I consider not using delimiters a capital sin that should be punished by hanging, firing squad, and fire. Preferably all three of them.

I'm sorry for the rant, but I take back nothing of what I said. I understand if someone does not like delimiters in the code. But when it starts to hurt your wallet instead of your eyes, you start reconsidering your taste regarding what looks "nice".

I have tried the script, the "simplified" version on Postgres 11 and 12 and did work as advertised. To make it actually do something, I created several test tables

	Code:
	CREATE TABLE test_script1(a INT, created TIMESTAMP DEFAULT clock_timestamp()); CREATE TABLE test_script2(a INT, created TIMESTAMP DEFAULT clock_timestamp()); CREATE TABLE test_script3(a INT, created TIMESTAMP DEFAULT clock_timestamp());

and it found and changed them successfully. I then dropped and recreated test tables and tried the original script, and that worked fine for me too. I even tried something very silly

	Code:
	CREATE TABLE "test_script4;"(a INT, created TIMESTAMP DEFAULT clock_timestamp());

and that worked too with both scripts and both databases versions

What kind of SQL statement is executed before the anonymous DO block? Are you able to reproduce this issue if you run just the anonymous DO block without other statements?