diff --git a/backup/queries_table_defs.go b/backup/queries_table_defs.go index 3b5d1351..d759348a 100644 --- a/backup/queries_table_defs.go +++ b/backup/queries_table_defs.go @@ -602,10 +602,30 @@ func GetPartitionAlteredSchema(connectionPool *dbconn.DBConn) map[uint32][]Alter func GetTableStorage(connectionPool *dbconn.DBConn) (map[uint32]string, map[uint32]string) { gplog.Info("Getting storage information") + // A single reloption is stored as a "name=value" element and the WITH clause + // is built by joining them with ", ". A value that is not a plain token + // (lowercase letters, digits, '_' or '.') breaks that round trip: + // - PAX minmax_columns/cluster_columns carry a comma-separated column list, + // e.g. "minmax_columns=a,b"; the embedded comma is read as the separator + // between options and restore fails with `unrecognized parameter "b"`. + // - an identifier with uppercase or special characters is silently + // downcased ("minmax_columns=A" targets column "a"), and a value with a + // space is a syntax error ("minmax_columns=x y"). + // Single-quote the value of any option whose value is not a simple token so + // it survives verbatim on restore. Plain-token values (compresstype=zstd, + // compresslevel=5, fillfactor=70, checksum=true, ...) are emitted unchanged, + // so backup output for existing tables is byte-for-byte unaffected. WITH + // ORDINALITY preserves the original option order. query := fmt.Sprintf(` SELECT c.oid, quote_ident(t.spcname) AS tablespace, - array_to_string(reloptions, ', ') AS reloptions + array_to_string(ARRAY( + SELECT CASE WHEN substr(opt, position('=' in opt) + 1) ~ '^[a-z0-9_.]+$' + THEN opt + ELSE left(opt, position('=' in opt) - 1) || '=' || quote_literal(substr(opt, position('=' in opt) + 1)) + END + FROM unnest(reloptions) WITH ORDINALITY AS u(opt, n) + ORDER BY n), ', ') AS reloptions FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid LEFT JOIN pg_tablespace t ON t.oid = c.reltablespace diff --git a/integration/predata_table_defs_queries_test.go b/integration/predata_table_defs_queries_test.go index 79a5d4ed..b6160564 100644 --- a/integration/predata_table_defs_queries_test.go +++ b/integration/predata_table_defs_queries_test.go @@ -846,6 +846,59 @@ SET SUBPARTITION TEMPLATE Expect(result[oid]).To(Equal("blocksize=32768, compresslevel=0, compresstype=none, checksum=true")) } }) + It("single-quotes comma-containing storage option values so PAX multi-column options restore correctly ", func() { + // PAX minmax_columns/cluster_columns store a comma-separated column + // list as a single string value. If that value is emitted unquoted, + // the embedded comma is read as an option separator and the restored + // DDL fails with `unrecognized parameter ""`. The value + // must be single-quoted so it survives the round trip. + if !connectionPool.Version.IsCBDB() { + Skip("PAX access method is only available on Cloudberry") + } + testhelper.AssertQueryRuns(connectionPool, + "CREATE TABLE public.pax_mm(a int, b int, c int) USING pax WITH (minmax_columns='a,b', cluster_columns='a,b', cluster_type='lexical') DISTRIBUTED BY (a)") + defer testhelper.AssertQueryRuns(connectionPool, "DROP TABLE public.pax_mm") + oid := testutils.OidFromObjectName(connectionPool, "public", "pax_mm", backup.TYPE_RELATION) + + _, result := backup.GetTableStorage(connectionPool) + + // The comma-containing list values must be single-quoted ... + Expect(result[oid]).To(ContainSubstring("minmax_columns='a,b'")) + Expect(result[oid]).To(ContainSubstring("cluster_columns='a,b'")) + + // ... and the emitted WITH clause must restore verbatim. This asserts + // the wiring end to end: a failure to quote would make this CREATE fail. + testhelper.AssertQueryRuns(connectionPool, + fmt.Sprintf("CREATE TABLE public.pax_mm_restored(a int, b int, c int) USING pax WITH (%s) DISTRIBUTED BY (a)", result[oid])) + testhelper.AssertQueryRuns(connectionPool, "DROP TABLE public.pax_mm_restored") + }) + It("single-quotes storage option values that are not simple tokens (uppercase, spaces) so they restore correctly", func() { + // A storage option value that is not a plain token also breaks the + // round trip even without a comma: an uppercase identifier is silently + // downcased (minmax_columns=A targets column "a"), and a value with a + // space is a syntax error (bloomfilter_columns=x y). Both must be + // single-quoted so the value survives verbatim on restore. + if !connectionPool.Version.IsCBDB() { + Skip("PAX access method is only available on Cloudberry") + } + testhelper.AssertQueryRuns(connectionPool, + `CREATE TABLE public.pax_q(a int, "A" int, "x y" int) USING pax WITH (minmax_columns='A', bloomfilter_columns='x y') DISTRIBUTED BY (a)`) + defer testhelper.AssertQueryRuns(connectionPool, "DROP TABLE public.pax_q") + oid := testutils.OidFromObjectName(connectionPool, "public", "pax_q", backup.TYPE_RELATION) + + _, result := backup.GetTableStorage(connectionPool) + + // The non-token values must be single-quoted ... + Expect(result[oid]).To(ContainSubstring("minmax_columns='A'")) + Expect(result[oid]).To(ContainSubstring("bloomfilter_columns='x y'")) + + // ... and the emitted WITH clause must restore verbatim onto the same + // uppercase/spaced columns. Without quoting this CREATE would downcase + // "A" to a missing column or fail with a syntax error. + testhelper.AssertQueryRuns(connectionPool, + fmt.Sprintf(`CREATE TABLE public.pax_q_restored(a int, "A" int, "x y" int) USING pax WITH (%s) DISTRIBUTED BY (a)`, result[oid])) + testhelper.AssertQueryRuns(connectionPool, "DROP TABLE public.pax_q_restored") + }) }) Describe("GetTableInheritance", func() { child := backup.Relation{Schema: "public", Name: "child"}