Issue 386: Do not call quote_ident() within repack_one_database() (#387)

It isn't necessary to call `quote_ident()` within `repack_one_database()`. `tablespace_dest` is passed to `repack.create_table()`, which already calls `quote_ident()`. `quote_ident()` within `repack_one_database()` can return quoted `tablespace_dest`, calling `quote_ident()` second time will make unexpected tablespace name with surrounding quotes. Tests were added.
reorg · Mar 12, 2024 · 7da5302 · 7da5302
1 parent 6a3c282
commit 7da5302
Show file tree

Hide file tree

Showing 9 changed files with 260 additions and 6 deletions.
diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml
@@ -34,11 +34,8 @@ jobs:
       - name: Put pg_repack on PATH
         run: echo "$PWD/bin" >> $GITHUB_PATH
 
-      - name: Create testts directory
-        run: sudo -u postgres mkdir /tmp/testts
-
-      - name: Create testts tablespace
-        run: sudo -u postgres psql -c "CREATE TABLESPACE testts LOCATION '/tmp/testts'"
+      - name: Create tablespaces
+        run: bash regress/create_tablespaces.sh
 
       - name: Test on PostgreSQL ${{ matrix.pg }}
         run: pg-build-test

diff --git a/bin/pg_repack.c b/bin/pg_repack.c
@@ -783,7 +783,7 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
 		"SELECT t.*,"
 		" coalesce(v.tablespace, t.tablespace_orig) as tablespace_dest"
 		" FROM repack.tables t, "
-		" (VALUES (quote_ident($1::text))) as v (tablespace)"
+		" (VALUES ($1::text)) as v (tablespace)"
 		" WHERE ");
 
 	params[iparam++] = tablespace;

diff --git a/regress/create_tablespaces.sh b/regress/create_tablespaces.sh
@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+
+sudo -u postgres mkdir /tmp/testts /tmp/1testts /tmp/test\ ts /tmp/test\"ts
+
+sudo -u postgres psql -c "CREATE TABLESPACE testts LOCATION '/tmp/testts'"
+sudo -u postgres psql -c "CREATE TABLESPACE \"1testts\" LOCATION '/tmp/1testts'"
+sudo -u postgres psql -c "CREATE TABLESPACE \"test ts\" LOCATION '/tmp/test ts'"
+sudo -u postgres psql -c "CREATE TABLESPACE \"test\"\"ts\" LOCATION '/tmp/test\"ts'"
diff --git a/regress/expected/tablespace.out b/regress/expected/tablespace.out
@@ -245,3 +245,48 @@ ORDER BY relname;
 --using --indexes-only and --index option together
 \! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey
 ERROR: cannot specify --index (-i) and --table (-t)
+--check quote_ident() with 1testts tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace=1testts --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | 1testts
+ testts1_partial_idx | 1testts
+ testts1_pkey        | 1testts
+ testts1_with_idx    | 1testts
+(4 rows)
+
+--check quote_ident() with "test ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test ts
+ testts1_partial_idx | test ts
+ testts1_pkey        | test ts
+ testts1_with_idx    | test ts
+(4 rows)
+
+--check quote_ident() with "test""ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test\"ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test"ts
+ testts1_partial_idx | test"ts
+ testts1_pkey        | test"ts
+ testts1_with_idx    | test"ts
+(4 rows)
+
diff --git a/regress/expected/tablespace_1.out b/regress/expected/tablespace_1.out
@@ -245,3 +245,48 @@ ORDER BY relname;
 --using --indexes-only and --index option together
 \! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey
 ERROR: cannot specify --index (-i) and --table (-t)
+--check quote_ident() with 1testts tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace=1testts --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | 1testts
+ testts1_partial_idx | 1testts
+ testts1_pkey        | 1testts
+ testts1_with_idx    | 1testts
+(4 rows)
+
+--check quote_ident() with "test ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test ts
+ testts1_partial_idx | test ts
+ testts1_pkey        | test ts
+ testts1_with_idx    | test ts
+(4 rows)
+
+--check quote_ident() with "test""ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test\"ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test"ts
+ testts1_partial_idx | test"ts
+ testts1_pkey        | test"ts
+ testts1_with_idx    | test"ts
+(4 rows)
+
diff --git a/regress/expected/tablespace_2.out b/regress/expected/tablespace_2.out
@@ -245,3 +245,48 @@ ORDER BY relname;
 --using --indexes-only and --index option together
 \! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey
 ERROR: cannot specify --index (-i) and --table (-t)
+--check quote_ident() with 1testts tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace=1testts --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | 1testts
+ testts1_partial_idx | 1testts
+ testts1_pkey        | 1testts
+ testts1_with_idx    | 1testts
+(4 rows)
+
+--check quote_ident() with "test ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test ts
+ testts1_partial_idx | test ts
+ testts1_pkey        | test ts
+ testts1_with_idx    | test ts
+(4 rows)
+
+--check quote_ident() with "test""ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test\"ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test"ts
+ testts1_partial_idx | test"ts
+ testts1_pkey        | test"ts
+ testts1_with_idx    | test"ts
+(4 rows)
+
diff --git a/regress/expected/tablespace_3.out b/regress/expected/tablespace_3.out
@@ -245,3 +245,48 @@ ORDER BY relname;
 --using --indexes-only and --index option together
 \! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey
 ERROR: cannot specify --index (-i) and --table (-t)
+--check quote_ident() with 1testts tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace=1testts --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | 1testts
+ testts1_partial_idx | 1testts
+ testts1_pkey        | 1testts
+ testts1_with_idx    | 1testts
+(4 rows)
+
+--check quote_ident() with "test ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test ts
+ testts1_partial_idx | test ts
+ testts1_pkey        | test ts
+ testts1_with_idx    | test ts
+(4 rows)
+
+--check quote_ident() with "test""ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test\"ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test"ts
+ testts1_partial_idx | test"ts
+ testts1_pkey        | test"ts
+ testts1_with_idx    | test"ts
+(4 rows)
+
diff --git a/regress/expected/tablespace_4.out b/regress/expected/tablespace_4.out
@@ -245,3 +245,48 @@ ORDER BY relname;
 --using --indexes-only and --index option together
 \! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey
 ERROR: cannot specify --index (-i) and --table (-t)
+--check quote_ident() with 1testts tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace=1testts --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | 1testts
+ testts1_partial_idx | 1testts
+ testts1_pkey        | 1testts
+ testts1_with_idx    | 1testts
+(4 rows)
+
+--check quote_ident() with "test ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test ts
+ testts1_partial_idx | test ts
+ testts1_pkey        | test ts
+ testts1_with_idx    | test ts
+(4 rows)
+
+--check quote_ident() with "test""ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test\"ts" --moveidx
+INFO: repacking table "public.testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | test"ts
+ testts1_partial_idx | test"ts
+ testts1_pkey        | test"ts
+ testts1_with_idx    | test"ts
+(4 rows)
+
diff --git a/regress/sql/tablespace.sql b/regress/sql/tablespace.sql
@@ -147,3 +147,27 @@ ORDER BY relname;
 
 --using --indexes-only and --index option together
 \! pg_repack --dbname=contrib_regression --table=testts1 --only-indexes --index=testts1_pkey
+
+--check quote_ident() with 1testts tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace=1testts --moveidx
+
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+
+--check quote_ident() with "test ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test ts" --moveidx
+
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+
+--check quote_ident() with "test""ts" tablespace
+\! pg_repack --dbname=contrib_regression --table=testts1 --tablespace="test\"ts" --moveidx
+
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;