From 0bcbf6708a3cc9b5b036d467ee1deb7f8a9fa4c5 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Fri, 28 Jun 2019 13:49:15 +0200 Subject: [PATCH 01/12] Remove usage of temporary tables --- scripts-available/CDB_SyncTable.sql | 72 ++++++++++++----------------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index 521495a..c4e49ca 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -67,6 +67,19 @@ AS $$ SELECT format('%s_%s_%s', prefix, txid_current(), (random()*1000000)::int)::NAME; $$ LANGUAGE sql VOLATILE PARALLEL UNSAFE; +/* + Given a table name and an array of column names, + return array of column names qualified with the table name + + Example of usage: + + SELECT cartodb.__CDB_QualifyColumns('t', ARRAY['a','b']); --> ARRAY['t.a','t.b'] + +*/ +CREATE OR REPLACE FUNCTION cartodb.__CDB_QualifyColumns(tablename TEXT, colnames TEXT[]) RETURNS TEXT[] AS +$$ + SELECT array_agg(tablename || '.' || _colname) from unnest(colnames) _colname; +$$ LANGUAGE sql IMMUTABLE PARALLEL SAFE; /* A Table Syncer @@ -88,7 +101,8 @@ DECLARE fq_dest_table TEXT; colnames TEXT[]; - quoted_colnames TEXT; + dst_colnames TEXT; + src_colnames TEXT; src_hash_table_name NAME; dst_hash_table_name NAME; @@ -113,35 +127,12 @@ BEGIN -- Get the list of columns from the source table, excluding skip_cols SELECT ARRAY(SELECT quote_ident(c) FROM cartodb._CDB_GetColumns(src_table) as c EXCEPT SELECT unnest(skip_cols)) INTO colnames; - quoted_colnames := array_to_string(colnames, ','); - - src_hash_table_name := cartodb.__CDB_GenerateUniqueName('src_sync'); - dst_hash_table_name := cartodb.__CDB_GenerateUniqueName('dst_sync'); - - EXECUTE format('CREATE TEMP TABLE %I(cartodb_id BIGINT, hash TEXT) ON COMMIT DROP', src_hash_table_name); - EXECUTE format('CREATE TEMP TABLE %I(cartodb_id BIGINT, hash TEXT) ON COMMIT DROP', dst_hash_table_name); - - -- Compute hash tables for src_table and dst_table h[cartodb_id] = hash(row) - -- It'll take the form of a temp table with an index (easy to run set operations) - t := clock_timestamp(); - EXECUTE format('INSERT INTO %I SELECT cartodb_id, md5(ROW(%s)::text) hash FROM %I', src_hash_table_name, quoted_colnames, src_table); - EXECUTE format('INSERT INTO %I SELECT cartodb_id, md5(ROW(%s)::text) hash FROM %s', dst_hash_table_name, quoted_colnames, fq_dest_table); - RAISE DEBUG 'Populate hash tables time (s): %', clock_timestamp() - t; - - -- Create indexes - -- We use hash indexes as they are fit for id comparison. - t := clock_timestamp(); - EXECUTE format('CREATE INDEX ON %I USING HASH (cartodb_id)', src_hash_table_name); - EXECUTE format('CREATE INDEX ON %I USING HASH (cartodb_id)', dst_hash_table_name); - RAISE DEBUG 'Index creation on hash tables time (s): %', clock_timestamp() - t; -- Deal with deleted rows: ids in dest but not in source t := clock_timestamp(); EXECUTE format( - 'DELETE FROM %s WHERE cartodb_id IN (SELECT cartodb_id FROM %I EXCEPT SELECT cartodb_id FROM %I)', - fq_dest_table, - dst_hash_table_name, - src_hash_table_name); + 'DELETE FROM %1$s _dst WHERE NOT EXISTS (SELECT * FROM %2$I _src WHERE _src.cartodb_id=_dst.cartodb_id)', + fq_dest_table, src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; RAISE NOTICE 'DELETED % row(s)', num_rows; RAISE DEBUG 'DELETE time (s): %', clock_timestamp() - t; @@ -149,29 +140,26 @@ BEGIN -- Deal with inserted rows: ids in source but not in dest t := clock_timestamp(); EXECUTE format(' - INSERT INTO %1$s (cartodb_id,%2$s) - SELECT h.cartodb_id,%2$s FROM (SELECT cartodb_id FROM %3$I EXCEPT SELECT cartodb_id FROM %4$I) h - LEFT JOIN %5$I s ON s.cartodb_id = h.cartodb_id; - ', fq_dest_table, quoted_colnames, src_hash_table_name, dst_hash_table_name, src_table); + INSERT INTO %1$s(cartodb_id, %2$s) + SELECT cartodb_id, %2$s FROM %3$I _src WHERE NOT EXISTS (SELECT * FROM %1$s _dst WHERE _src.cartodb_id=_dst.cartodb_id) + ', fq_dest_table, array_to_string(colnames, ','), src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; RAISE NOTICE 'INSERTED % row(s)', num_rows; RAISE DEBUG 'INSERT time (s): %', clock_timestamp() - t; -- Deal with modified rows: ids in source and dest but different hashes t := clock_timestamp(); - update_set_clause := cartodb.__CDB_GetUpdateSetClause(colnames, 'changed'); + update_set_clause := cartodb.__CDB_GetUpdateSetClause(colnames, '_changed'); + dst_colnames := array_to_string(cartodb.__CDB_QualifyColumns('_dst', colnames), ','); + src_colnames := array_to_string(cartodb.__CDB_QualifyColumns('_src', colnames), ','); EXECUTE format(' - UPDATE %1$s dst SET %2$s - FROM ( - SELECT * - FROM %3$s src - WHERE cartodb_id IN - (SELECT sh.cartodb_id FROM %4$I sh - LEFT JOIN %5$I dh ON sh.cartodb_id = dh.cartodb_id - WHERE sh.hash <> dh.hash) - ) changed - WHERE dst.cartodb_id = changed.cartodb_id; - ', fq_dest_table, update_set_clause, src_table, src_hash_table_name, dst_hash_table_name); + UPDATE %1$s _update SET %2$s + FROM ( + SELECT _src.* FROM %3$s _src JOIN %1$s _dst ON (_dst.cartodb_id = _src.cartodb_id) + WHERE md5(ROW(%4$s)::text) <> md5(ROW(%5$s)::text) + ) _changed + WHERE _update.cartodb_id = _changed.cartodb_id; + ', fq_dest_table, update_set_clause, src_table, dst_colnames, src_colnames); GET DIAGNOSTICS num_rows = ROW_COUNT; RAISE NOTICE 'MODIFIED % row(s)', num_rows; RAISE DEBUG 'UPDATE time (s): %', clock_timestamp() - t; From 46518834548dc45d22a3e53007494a2fe91eff14 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 2 Jul 2019 17:43:13 +0200 Subject: [PATCH 02/12] Fix order of test results --- test/CDB_SyncTableTest.sql | 4 ++-- test/CDB_SyncTableTest_expect | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/CDB_SyncTableTest.sql b/test/CDB_SyncTableTest.sql index a945640..57091c2 100644 --- a/test/CDB_SyncTableTest.sql +++ b/test/CDB_SyncTableTest.sql @@ -50,8 +50,8 @@ UPDATE test_sync_dest SET the_geom = cartodb.CDB_LatLng(lat, lon); -- A "gecodin \set QUIET off SET client_min_messages TO notice; SELECT cartodb.CDB_SyncTable('test_sync_source', 'public', 'test_sync_dest', '{the_geom, the_geom_webmercator}'); -SELECT * FROM test_sync_source; -SELECT * FROM test_sync_dest; +SELECT * FROM test_sync_source ORDER BY cartodb_id; +SELECT * FROM test_sync_dest ORDER BY cartodb_id; -- Cleanup diff --git a/test/CDB_SyncTableTest_expect b/test/CDB_SyncTableTest_expect index ffe8ee2..50157a7 100644 --- a/test/CDB_SyncTableTest_expect +++ b/test/CDB_SyncTableTest_expect @@ -52,10 +52,10 @@ NOTICE: MODIFIED 0 row(s) 1|||1|1|foo 2|||2|2|bar -5|||5|5|sandia 4|||4|4|cantaloupe +5|||5|5|sandia 1|0101000020E6100000000000000000F03F000000000000F03F|0101000020110F0000DB0B4ADA772DFB402B432E49D22DFB40|1|1|foo 2|0101000020E610000000000000000000400000000000000040|0101000020110F00003C0C4ADA772D0B4177F404ABE12E0B41|2|2|bar -5|0101000020E610000000000000000014400000000000001440|0101000020110F000099476EE86AFC20413E7EB983F2012141|5|5|sandia 4|0101000020E610000000000000000010400000000000001040|0101000020110F00003C0C4ADA772D1B4160AB497020331B41|4|4|cantaloupe +5|0101000020E610000000000000000014400000000000001440|0101000020110F000099476EE86AFC20413E7EB983F2012141|5|5|sandia ROLLBACK From 65483743b4e76ee1e96fce32b0e3496e256c88d8 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 2 Jul 2019 17:44:06 +0200 Subject: [PATCH 03/12] Remove unused vars --- scripts-available/CDB_SyncTable.sql | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index d7dd14f..908c4c0 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -104,9 +104,6 @@ DECLARE dst_colnames TEXT; src_colnames TEXT; - src_hash_table_name NAME; - dst_hash_table_name NAME; - update_set_clause TEXT; num_rows BIGINT; From d8c840d1264da15a20f548251fccf79dc5c23349 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 2 Jul 2019 18:14:04 +0200 Subject: [PATCH 04/12] Quote table and column names when necessary Also use type NAME when appropriate. Note that quoted column names are not NAMES (may be longer). --- scripts-available/CDB_SyncTable.sql | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index 908c4c0..296b0da 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -69,16 +69,30 @@ $$ LANGUAGE sql VOLATILE PARALLEL UNSAFE; /* Given a table name and an array of column names, - return array of column names qualified with the table name + return array of column names qualified with the table name and quoted when necessary Example of usage: - SELECT cartodb.__CDB_QualifyColumns('t', ARRAY['a','b']); --> ARRAY['t.a','t.b'] + SELECT @extschema@.__CDB_QualifyColumns('t', ARRAY['a','b-1']); --> ARRAY['t.a','t."b-1"'] */ -CREATE OR REPLACE FUNCTION cartodb.__CDB_QualifyColumns(tablename TEXT, colnames TEXT[]) RETURNS TEXT[] AS +CREATE OR REPLACE FUNCTION @extschema@.__CDB_QualifyColumns(tablename NAME, colnames NAME[]) RETURNS TEXT[] AS $$ - SELECT array_agg(tablename || '.' || _colname) from unnest(colnames) _colname; + SELECT array_agg(quote_ident(tablename) || '.' || quote_ident(_colname)) from unnest(colnames) _colname; +$$ LANGUAGE sql IMMUTABLE PARALLEL SAFE; + +/* + Given an array of column names, + return array of column names quoted when necessary + + Example of usage: + + SELECT @extschema@.__CDB_QuoteColumns(ARRAY['a','b-1']); --> ARRAY['a','"b-1"'] + +*/ +CREATE OR REPLACE FUNCTION @extschema@.__CDB_QuoteColumns(colnames NAME[]) RETURNS TEXT[] AS +$$ + SELECT array_agg(quote_ident(_colname)) from unnest(colnames) _colname; $$ LANGUAGE sql IMMUTABLE PARALLEL SAFE; /* @@ -139,7 +153,7 @@ BEGIN EXECUTE format(' INSERT INTO %1$s(cartodb_id, %2$s) SELECT cartodb_id, %2$s FROM %3$I _src WHERE NOT EXISTS (SELECT * FROM %1$s _dst WHERE _src.cartodb_id=_dst.cartodb_id) - ', fq_dest_table, array_to_string(colnames, ','), src_table); + ', fq_dest_table, array_to_string(@extschema@.__CDB_QuoteColumns(colnames), ','), src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; RAISE NOTICE 'INSERTED % row(s)', num_rows; RAISE DEBUG 'INSERT time (s): %', clock_timestamp() - t; @@ -147,8 +161,8 @@ BEGIN -- Deal with modified rows: ids in source and dest but different hashes t := clock_timestamp(); update_set_clause := @extschema@.__CDB_GetUpdateSetClause(colnames, '_changed'); - dst_colnames := array_to_string(cartodb.__CDB_QualifyColumns('_dst', colnames), ','); - src_colnames := array_to_string(cartodb.__CDB_QualifyColumns('_src', colnames), ','); + dst_colnames := array_to_string(@extschema@.__CDB_QualifyColumns('_dst', colnames), ','); + src_colnames := array_to_string(@extschema@.__CDB_QualifyColumns('_src', colnames), ','); EXECUTE format(' UPDATE %1$s _update SET %2$s FROM ( From f5f18ca57cd6e4eed0eee6f171b61b1df9184f06 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 2 Jul 2019 18:14:18 +0200 Subject: [PATCH 05/12] Fix the order of the columns --- scripts-available/CDB_SyncTable.sql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index 296b0da..fa664af 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -21,7 +21,8 @@ AS $$ LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE c.oid = src_table::oid AND pg_catalog.pg_table_is_visible(c.oid) - ); + ) + ORDER BY a.attnum; $$ LANGUAGE sql STABLE PARALLEL UNSAFE; From 5963c67376d749872404ee62cff76e2eb236fedd Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 3 Jul 2019 16:16:37 +0200 Subject: [PATCH 06/12] Order the columns of a cartodbfied table consistently The final order of the columns of a cartodbfied table wasn't uniquely specified, so could vary across PG versions. This was a problem in particular for having deterministic test results. --- scripts-available/CDB_CartodbfyTable.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 9bfb93c..c54baf2 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -1071,7 +1071,7 @@ BEGIN -- by selecting their names into an array and -- joining the array with a comma SELECT - ',' || array_to_string(array_agg(Format('%I',a.attname)),',') AS column_name_sql, + ',' || array_to_string(array_agg(Format('%I',a.attname) ORDER BY a.attnum),',') AS column_name_sql, Count(*) AS count INTO rec FROM pg_class c From 2e1fe2933cb29d2e69d126ef01acac68ce5ceecc Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 3 Jul 2019 16:56:29 +0200 Subject: [PATCH 07/12] Fix whitespace --- scripts-available/CDB_SyncTable.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index fa664af..322ec37 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -138,7 +138,7 @@ BEGIN skip_cols := skip_cols || '{cartodb_id}'; -- Get the list of columns from the source table, excluding skip_cols - SELECT ARRAY(SELECT quote_ident(c) FROM @extschema@._CDB_GetColumns(src_table) as c EXCEPT SELECT unnest(skip_cols)) INTO colnames; + SELECT ARRAY(SELECT quote_ident(c) FROM @extschema@._CDB_GetColumns(src_table) as c EXCEPT SELECT unnest(skip_cols)) INTO colnames; -- Deal with deleted rows: ids in dest but not in source t := clock_timestamp(); From c07784566a12534a244b8690fbdd4a5e97d06a1b Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 3 Jul 2019 17:02:36 +0200 Subject: [PATCH 08/12] NEWS for next release --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 8461371..be270e3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,6 @@ +x.y.z (YYYY-MM-DD) +* Avoid temporary tables creation in CDB_SyncTable (#366) + 0.28.0 (2019-07-01) * New function CDB_SyncTable (#355) From 7bdee5c13e4ac2a13c0ef8b418ff287fff3996b5 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 3 Jul 2019 18:34:54 +0200 Subject: [PATCH 09/12] Avoid double-quoting Since dst_schema is a REGNAMESPACE, it is automatically quoted when casted to TEXT --- scripts-available/CDB_SyncTable.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index 322ec37..122db9a 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -127,7 +127,7 @@ DECLARE t timestamptz; BEGIN -- If the destination table does not exist, just copy the source table - fq_dest_table := format('%I.%I', dst_schema, dst_table); + fq_dest_table := format('%s.%I', dst_schema, dst_table); EXECUTE format('CREATE TABLE IF NOT EXISTS %s as TABLE %I', fq_dest_table, src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; IF num_rows > 0 THEN From 2beabfced6c15d514bf6ae9d2c5143c3940300b4 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 3 Jul 2019 18:48:49 +0200 Subject: [PATCH 10/12] Add test for double schema-quoting --- test/CDB_SyncTableTest.sql | 6 ++++++ test/CDB_SyncTableTest_expect | 2 ++ 2 files changed, 8 insertions(+) diff --git a/test/CDB_SyncTableTest.sql b/test/CDB_SyncTableTest.sql index 57091c2..53d1362 100644 --- a/test/CDB_SyncTableTest.sql +++ b/test/CDB_SyncTableTest.sql @@ -53,6 +53,12 @@ SELECT cartodb.CDB_SyncTable('test_sync_source', 'public', 'test_sync_dest', '{t SELECT * FROM test_sync_source ORDER BY cartodb_id; SELECT * FROM test_sync_dest ORDER BY cartodb_id; +\echo 'It will work with schemas that need quoting' +\set QUIET on +SET client_min_messages TO error; +CREATE SCHEMA "sch-ema"; +\set QUIET off +SELECT cartodb.CDB_SyncTable('test_sync_source', 'sch-ema', 'test_sync_dest'); -- Cleanup ROLLBACK; diff --git a/test/CDB_SyncTableTest_expect b/test/CDB_SyncTableTest_expect index 50157a7..5963a01 100644 --- a/test/CDB_SyncTableTest_expect +++ b/test/CDB_SyncTableTest_expect @@ -58,4 +58,6 @@ NOTICE: MODIFIED 0 row(s) 2|0101000020E610000000000000000000400000000000000040|0101000020110F00003C0C4ADA772D0B4177F404ABE12E0B41|2|2|bar 4|0101000020E610000000000000000010400000000000001040|0101000020110F00003C0C4ADA772D1B4160AB497020331B41|4|4|cantaloupe 5|0101000020E610000000000000000014400000000000001440|0101000020110F000099476EE86AFC20413E7EB983F2012141|5|5|sandia +It will work with schemas that need quoting + ROLLBACK From cb353ec6a8638046f9f143577d16916fa75068bf Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 4 Jul 2019 12:47:01 +0200 Subject: [PATCH 11/12] Add tests for quoted table & column names --- test/CDB_SyncTableTest.sql | 32 +++++++++++++++++++++++++++++++- test/CDB_SyncTableTest_expect | 31 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/test/CDB_SyncTableTest.sql b/test/CDB_SyncTableTest.sql index 53d1362..fc2711e 100644 --- a/test/CDB_SyncTableTest.sql +++ b/test/CDB_SyncTableTest.sql @@ -57,8 +57,38 @@ SELECT * FROM test_sync_dest ORDER BY cartodb_id; \set QUIET on SET client_min_messages TO error; CREATE SCHEMA "sch-ema"; +CREATE TABLE "test_sync_source2" AS SELECT * FROM test_sync_source; \set QUIET off -SELECT cartodb.CDB_SyncTable('test_sync_source', 'sch-ema', 'test_sync_dest'); +SELECT cartodb.CDB_SyncTable('test_sync_source2', 'sch-ema', 'test_sync_dest'); +INSERT INTO test_sync_source2(cartodb_id, lat, lon, name) VALUES (6, 6.0, 6.0, 'papaya'); +DELETE FROM test_sync_source2 WHERE cartodb_id = 4; +UPDATE test_sync_source2 SET lat = 2.5 WHERE cartodb_id = 2; +SET client_min_messages TO notice; +SELECT cartodb.CDB_SyncTable('test_sync_source2', 'sch-ema', 'test_sync_dest'); + +\echo 'It will work with table names that need quoting' +\set QUIET on +SET client_min_messages TO error; +CREATE TABLE "test-sync-source" AS SELECT * FROM test_sync_source; +\set QUIET off +SELECT cartodb.CDB_SyncTable('test-sync-source', 'public', 'test-sync-dest'); +INSERT INTO "test-sync-source"(cartodb_id, lat, lon, name) VALUES (6, 6.0, 6.0, 'papaya'); +DELETE FROM "test-sync-source" WHERE cartodb_id = 4; +UPDATE "test-sync-source" SET lat = 2.5 WHERE cartodb_id = 2; +SET client_min_messages TO notice; +SELECT cartodb.CDB_SyncTable('test-sync-source', 'public', 'test-sync-dest'); + +\echo 'It will work with column names that need quoting' +\set QUIET on +SET client_min_messages TO error; +ALTER TABLE test_sync_source ADD COLUMN "a-column" int; +\set QUIET off +SELECT cartodb.CDB_SyncTable('test_sync_source', 'public', 'test_sync_dest2'); +INSERT INTO test_sync_source(cartodb_id, lat, lon, name) VALUES (6, 6.0, 6.0, 'papaya'); +DELETE FROM test_sync_source WHERE cartodb_id = 4; +UPDATE test_sync_source SET lat = 2.5 WHERE cartodb_id = 2; +SET client_min_messages TO notice; +SELECT cartodb.CDB_SyncTable('test_sync_source', 'public', 'test_sync_dest2'); -- Cleanup ROLLBACK; diff --git a/test/CDB_SyncTableTest_expect b/test/CDB_SyncTableTest_expect index 5963a01..01cb358 100644 --- a/test/CDB_SyncTableTest_expect +++ b/test/CDB_SyncTableTest_expect @@ -60,4 +60,35 @@ NOTICE: MODIFIED 0 row(s) 5|0101000020E610000000000000000014400000000000001440|0101000020110F000099476EE86AFC20413E7EB983F2012141|5|5|sandia It will work with schemas that need quoting +INSERT 0 1 +DELETE 1 +UPDATE 1 +SET +NOTICE: relation "test_sync_dest" already exists, skipping +NOTICE: DELETED 1 row(s) +NOTICE: INSERTED 1 row(s) +NOTICE: MODIFIED 1 row(s) + +It will work with table names that need quoting + +INSERT 0 1 +DELETE 1 +UPDATE 1 +SET +NOTICE: relation "test-sync-dest" already exists, skipping +NOTICE: DELETED 1 row(s) +NOTICE: INSERTED 1 row(s) +NOTICE: MODIFIED 1 row(s) + +It will work with column names that need quoting + +INSERT 0 1 +DELETE 1 +UPDATE 1 +SET +NOTICE: relation "test_sync_dest2" already exists, skipping +NOTICE: DELETED 1 row(s) +NOTICE: INSERTED 1 row(s) +NOTICE: MODIFIED 1 row(s) + ROLLBACK From dbd403a2f6f881e53d6abdffa6c2cb042b593758 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 4 Jul 2019 12:47:58 +0200 Subject: [PATCH 12/12] Fix cases of double-quoting identifiers --- scripts-available/CDB_SyncTable.sql | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/scripts-available/CDB_SyncTable.sql b/scripts-available/CDB_SyncTable.sql index 122db9a..9d35788 100644 --- a/scripts-available/CDB_SyncTable.sql +++ b/scripts-available/CDB_SyncTable.sql @@ -71,29 +71,17 @@ $$ LANGUAGE sql VOLATILE PARALLEL UNSAFE; /* Given a table name and an array of column names, return array of column names qualified with the table name and quoted when necessary + tablename and colnames should be properly quoted, and for this reason the type NAME is not + used for them (with quotes they could exceed the maximum identifier length) Example of usage: - SELECT @extschema@.__CDB_QualifyColumns('t', ARRAY['a','b-1']); --> ARRAY['t.a','t."b-1"'] + SELECT @extschema@.__CDB_QualifyColumns('t', ARRAY['a','"b-1"']); --> ARRAY['t.a','t."b-1"'] */ CREATE OR REPLACE FUNCTION @extschema@.__CDB_QualifyColumns(tablename NAME, colnames NAME[]) RETURNS TEXT[] AS $$ - SELECT array_agg(quote_ident(tablename) || '.' || quote_ident(_colname)) from unnest(colnames) _colname; -$$ LANGUAGE sql IMMUTABLE PARALLEL SAFE; - -/* - Given an array of column names, - return array of column names quoted when necessary - - Example of usage: - - SELECT @extschema@.__CDB_QuoteColumns(ARRAY['a','b-1']); --> ARRAY['a','"b-1"'] - -*/ -CREATE OR REPLACE FUNCTION @extschema@.__CDB_QuoteColumns(colnames NAME[]) RETURNS TEXT[] AS -$$ - SELECT array_agg(quote_ident(_colname)) from unnest(colnames) _colname; + SELECT array_agg(tablename || '.' || _colname) from unnest(colnames) _colname; $$ LANGUAGE sql IMMUTABLE PARALLEL SAFE; /* @@ -128,7 +116,7 @@ DECLARE BEGIN -- If the destination table does not exist, just copy the source table fq_dest_table := format('%s.%I', dst_schema, dst_table); - EXECUTE format('CREATE TABLE IF NOT EXISTS %s as TABLE %I', fq_dest_table, src_table); + EXECUTE format('CREATE TABLE IF NOT EXISTS %s as TABLE %s', fq_dest_table, src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; IF num_rows > 0 THEN RAISE NOTICE 'INSERTED % row(s)', num_rows; @@ -143,7 +131,7 @@ BEGIN -- Deal with deleted rows: ids in dest but not in source t := clock_timestamp(); EXECUTE format( - 'DELETE FROM %1$s _dst WHERE NOT EXISTS (SELECT * FROM %2$I _src WHERE _src.cartodb_id=_dst.cartodb_id)', + 'DELETE FROM %1$s _dst WHERE NOT EXISTS (SELECT * FROM %2$s _src WHERE _src.cartodb_id=_dst.cartodb_id)', fq_dest_table, src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; RAISE NOTICE 'DELETED % row(s)', num_rows; @@ -153,8 +141,8 @@ BEGIN t := clock_timestamp(); EXECUTE format(' INSERT INTO %1$s(cartodb_id, %2$s) - SELECT cartodb_id, %2$s FROM %3$I _src WHERE NOT EXISTS (SELECT * FROM %1$s _dst WHERE _src.cartodb_id=_dst.cartodb_id) - ', fq_dest_table, array_to_string(@extschema@.__CDB_QuoteColumns(colnames), ','), src_table); + SELECT cartodb_id, %2$s FROM %3$s _src WHERE NOT EXISTS (SELECT * FROM %1$s _dst WHERE _src.cartodb_id=_dst.cartodb_id) + ', fq_dest_table, array_to_string(colnames, ','), src_table); GET DIAGNOSTICS num_rows = ROW_COUNT; RAISE NOTICE 'INSERTED % row(s)', num_rows; RAISE DEBUG 'INSERT time (s): %', clock_timestamp() - t;