CDB_RegenerateTable: Better behaviour with transactions

scripts-available/CDB_RegenerateTable.sql

@@ -22,12 +22,12 @@ AS $$
 
     # NOTE: We always use -s so data is never dumped!
     # That would be a security issue that we would need to deal with (and we currently do not need it)
-    process_parameters = ["pg_dump", "-s", "-t", full_tablename_string, database_name_string]
+    process_parameters = ["pg_dump", "-s", "--lock-wait-timeout", "1000", "-t", full_tablename_string, database_name_string]
 
     proc = subprocess.Popen(process_parameters, stdout=subprocess.PIPE, shell=False)
     (out, err) = proc.communicate()
-    if (err):
-        plpy.error(err)
+    if (err or not out):
+        plpy.error('Could not get table properties')
 
     line = out.decode("utf-8")
     lines = line.rsplit(";\n", -1)
@@ -38,9 +38,9 @@ AS $$
         sublines = [line.rstrip() for line in sublines]
         sublines = [line for line in sublines if line]
         sublines = [line for line in sublines if not line.startswith('--')]
-        # We need to force all setting changes to be local to keep the environment clean
-        sublines = [re.sub(r'^SET ', 'SET LOCAL ', line) for line in sublines]
-        sublines = [re.sub(r'(.*)(pg_catalog.set_config.*)(false)(.*)', r'\1\2true\4', line) for line in sublines]
+        sublines = [line for line in sublines if not line.lower().startswith('set ')]
+        sublines = [line for line in sublines if line.lower().find('pg_catalog.set_config(') == -1]
+
         if len(sublines):
             clean_lines.append("".join(sublines))
 
@@ -49,7 +49,7 @@ $$
 LANGUAGE @@plpythonu@@ VOLATILE PARALLEL UNSAFE;
 
 -- Returns a list of queries that can be used to regenerate the structure of a table
--- The query to create the table and the config set by pg_dump are removed
+-- The query to create the table is not included
 -- The optional parameter **ignore_cartodbfication** will remove queries related to the cartodbfication of the table
 CREATE OR REPLACE FUNCTION @extschema@.CDB_GetTableQueries(tableoid OID, ignore_cartodbfication BOOL DEFAULT false)
 RETURNS text[]
@@ -68,16 +68,12 @@ BEGIN
         EXECUTE FORMAT('
             SELECT array_agg(a)
                 FROM unnest(@extschema@.__CDB_RegenerateTable_Get_Commands(%L)) a
-                WHERE   a NOT SIMILAR TO ''CREATE TABLE%%'' AND
-                        a NOT SIMILAR TO ''SET%%'' AND
-                        a NOT SIMILAR TO (''%%pg_catalog.set_config%%'');', tableoid) INTO queries;
+                WHERE   a NOT SIMILAR TO ''CREATE TABLE%%'';', tableoid) INTO queries;
     ELSE
         EXECUTE FORMAT('
             SELECT array_agg(a)
                 FROM unnest(@extschema@.__CDB_RegenerateTable_Get_Commands(%L)) a
                 WHERE   a NOT SIMILAR TO ''CREATE TABLE%%'' AND
-                        a NOT SIMILAR TO ''SET%%'' AND
-                        a NOT SIMILAR TO (''%%pg_catalog.set_config%%'') AND
                         a NOT SIMILAR TO (''%%PRIMARY KEY \(cartodb_id\)%%'') AND
                         a NOT SIMILAR TO (''%%cartodb_id_seq%%'') AND
                         a NOT SIMILAR TO (''%%track_updates%%'') AND

test/CDB_RegenerateTable.sql

@@ -160,20 +160,46 @@ SELECT cartodb.CDB_RegenerateTable('testtable'::regclass::oid);
 CREATE ROLE cdb_regenerate_tester LOGIN PASSWORD 'cdb_regenerate_pass';
 GRANT CONNECT ON DATABASE contrib_regression TO cdb_regenerate_tester;
 GRANT SELECT ON testtable TO cdb_regenerate_tester;
+
 \c contrib_regression cdb_regenerate_tester
+\set QUIET on
+SET client_min_messages TO error;
+\set VERBOSITY terse
+\set QUIET off
+
 SELECT * FROM testtable ORDER BY cartodb_id DESC;
+
 \c contrib_regression postgres
+\set QUIET on
+SET client_min_messages TO error;
+\set VERBOSITY terse
+\set QUIET off
 
 SELECT cartodb.CDB_RegenerateTable('testtable'::regclass::oid);
 
 \c contrib_regression cdb_regenerate_tester
+\set QUIET on
+SET client_min_messages TO error;
+\set VERBOSITY terse
+\set QUIET off
+
 SELECT * FROM testtable ORDER BY cartodb_id DESC;
+
 \c contrib_regression postgres
+\set QUIET on
+SET client_min_messages TO error;
+\set VERBOSITY terse
+\set QUIET off
 
 \echo '## Test calling with read only access (should fail)'
 \c contrib_regression cdb_regenerate_tester
 SELECT cartodb.CDB_RegenerateTable('testtable'::regclass::oid);
+
 \c contrib_regression postgres
+\set QUIET on
+SET client_min_messages TO error;
+\set VERBOSITY terse
+\set QUIET off
 
 \echo '## Test partitioned table'
 CREATE TABLE measurement (
@@ -217,6 +243,26 @@ ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_ca
 
 SELECT cartodb.CDB_GetTableQueries_TestHelper('measurement'::regclass::oid, ignore_cartodbfication := false);
 
+\echo '## Test transaction with truncate'
+BEGIN;
+    TRUNCATE TABLE testtable;
+    SELECT CDB_RegenerateTable('public.testtable'::regclass);
+COMMIT;
+
+\echo '## Test transaction with delete'
+BEGIN;
+    DELETE FROM testtable WHERE true;
+    SELECT CDB_RegenerateTable('public.testtable'::regclass);
+COMMIT;
+
+\echo '## Test transaction with delete + cartodbfy'
+BEGIN;
+    INSERT INTO testtable(stable,c1,c2,c3,c4) VALUES (1,2,3,4,5), (2,3,4,5,6), (3,4,5,6,7);
+    DELETE FROM testtable WHERE true;
+    SELECT CDB_RegenerateTable('public.testtable'::regclass);
+    SELECT CDB_CartodbfyTable('public'::TEXT, 'public.testtable'::REGCLASS);
+COMMIT;
+
 \echo '## teardown'
 
 DROP TABLE measurement CASCADE;

test/CDB_RegenerateTable_expect

@@ -178,6 +178,23 @@ logdate|date||not null|
 peaktemp|integer|||
 unitsales|integer|||
 ERROR:  CDB_GetTableQueries does not support the parent of partitioned tables
+## Test transaction with truncate
+BEGIN
+TRUNCATE TABLE
+ERROR:  plpy.Error: Could not get table properties
+ROLLBACK
+## Test transaction with delete
+BEGIN
+DELETE 3
+
+COMMIT
+## Test transaction with delete + cartodbfy
+BEGIN
+INSERT 0 3
+DELETE 3
+
+testtable
+COMMIT
 ## teardown
 DROP TABLE
 DROP TABLE