From d2b34b3264f5ebaa0d8739cb54e9fe9b33123dc7 Mon Sep 17 00:00:00 2001 From: talmacschen-arch Date: Sat, 31 Oct 2026 23:52:30 +0800 Subject: [PATCH] Quote non-token storage option values for materialized views GetAllViews builds a materialized view's WITH clause by joining pg_class.reloptions ("name=value" elements) with ", ", the same way GetTableStorage did for tables. A value that is not a plain token (lowercase letters, digits, '_' or '.') does not survive that round trip. The prime case is a PAX materialized view, whose minmax_columns/cluster_columns carry a comma-separated column list: CREATE MATERIALIZED VIEW ... USING pax WITH (minmax_columns=a,b) AS ... ERROR: unrecognized parameter "b" (SQLSTATE 22023) The matview is then never created, and its ALTER ... OWNER cascades into "relation does not exist", so the matview is lost on restore. Uppercase or spaced option values break the same way. Render reloptions through a shared helper (relOptionsWithClause) used by both the GPDB6 and GPDB7+/Cloudberry matview queries. It single-quotes any value that is not a simple token (not matching ^[a-z0-9_.]+$), which covers the comma, uppercase and space cases, and leaves plain-token values (fillfactor=50, autovacuum_enabled=false, ...) byte-for-byte unchanged. It preserves the original " WITH (...)" vs "" (no reloptions) semantics via an explicit NULL check, and WITH ORDINALITY keeps the option order. Verified end to end on a live Cloudberry 2.1.0 cluster: a PAX matview with multi-column minmax_columns/cluster_columns now backs up as `WITH (minmax_columns='a,b', cluster_columns='a,b', ...)` and restores cleanly, while a plain heap matview with no reloptions is emitted without a WITH clause. Add an integration test asserting the quoted WITH clause recreates the matview verbatim. This is the materialized-view follow-up noted in the base-table fix (#127); the two share the same root cause but touch different queries. --- backup/queries_relations.go | 33 ++++++++++++++--- integration/predata_relations_queries_test.go | 36 +++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/backup/queries_relations.go b/backup/queries_relations.go index 34652a84..796a9721 100644 --- a/backup/queries_relations.go +++ b/backup/queries_relations.go @@ -384,6 +384,29 @@ func (v View) ObjectType() string { return toc.OBJ_VIEW } +// relOptionsWithClause returns a SQL expression that renders the given +// pg_class.reloptions column as a " WITH (...)" clause for a CREATE statement. +// A single reloption is stored as a "name=value" element; a value that is not a +// simple token (lowercase letters, digits, '_' or '.') is single-quoted so that +// comma-separated, uppercase or spaced values survive the backup/restore round +// trip. The prime example is a PAX materialized view's minmax_columns / +// cluster_columns, whose comma-separated column list would otherwise be read as +// the separator between options and fail restore with `unrecognized parameter`. +// Plain-token values (e.g. compresstype=zstd) are emitted unchanged, and the +// expression yields '' when reloptions is NULL. WITH ORDINALITY preserves the +// original option order. +func relOptionsWithClause(reloptionsCol string) string { + return fmt.Sprintf(`CASE WHEN %[1]s IS NULL THEN '' + ELSE ' WITH (' || 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(%[1]s) WITH ORDINALITY AS u(opt, n) + ORDER BY n), ', ') || ')' + END`, reloptionsCol) +} + // This function retrieves both regular views and materialized views. func GetAllViews(connectionPool *dbconn.DBConn) []View { columnDefs := GetColumnDefinitions(connectionPool) @@ -413,13 +436,15 @@ func GetAllViews(connectionPool *dbconn.DBConn) []View { // Materialized views were introduced in GPDB 7 and backported to GPDB 6.2. // Reloptions and tablespace added to pg_class in GPDB 6 + optionsExpr := relOptionsWithClause("c.reloptions") + version6Query := fmt.Sprintf(` SELECT c.oid AS oid, quote_ident(n.nspname) AS schema, quote_ident(c.relname) AS name, pg_get_viewdef(c.oid) AS definition, - coalesce(' WITH (' || array_to_string(c.reloptions, ', ') || ')', '') AS options, + %s AS options, coalesce(quote_ident(t.spcname), '') AS tablespace, c.relkind='m' AS ismaterialized FROM pg_class c @@ -427,7 +452,7 @@ func GetAllViews(connectionPool *dbconn.DBConn) []View { LEFT JOIN pg_tablespace t ON t.oid = c.reltablespace WHERE c.relkind IN ('m', 'v') AND %s - AND %s`, relationAndSchemaFilterClause(), ExtensionFilterClause("c")) + AND %s`, optionsExpr, relationAndSchemaFilterClause(), ExtensionFilterClause("c")) atLeast7Query := fmt.Sprintf(` SELECT @@ -435,7 +460,7 @@ func GetAllViews(connectionPool *dbconn.DBConn) []View { quote_ident(n.nspname) AS schema, quote_ident(c.relname) AS name, pg_get_viewdef(c.oid) AS definition, - coalesce(' WITH (' || array_to_string(c.reloptions, ', ') || ')', '') AS options, + %s AS options, coalesce(quote_ident(t.spcname), '') AS tablespace, c.relkind='m' AS ismaterialized, coalesce(quote_ident(am.amname), '') AS accessmethodname @@ -445,7 +470,7 @@ func GetAllViews(connectionPool *dbconn.DBConn) []View { LEFT JOIN pg_am am ON am.oid = c.relam WHERE c.relkind IN ('m', 'v') AND %s - AND %s`, relationAndSchemaFilterClause(), ExtensionFilterClause("c")) + AND %s`, optionsExpr, relationAndSchemaFilterClause(), ExtensionFilterClause("c")) query := "" if connectionPool.Version.IsGPDB() && connectionPool.Version.Before("6") { diff --git a/integration/predata_relations_queries_test.go b/integration/predata_relations_queries_test.go index a0fc446e..6d9c96a5 100644 --- a/integration/predata_relations_queries_test.go +++ b/integration/predata_relations_queries_test.go @@ -2,6 +2,7 @@ package integration import ( "database/sql" + "fmt" "math" "sort" "strconv" @@ -553,5 +554,40 @@ PARTITION BY LIST (gender) Expect(results).To(HaveLen(1)) structmatcher.ExpectStructsToMatchExcluding(&materialView, &results[0], "Oid", "ColumnDefs", "DistPolicy.Oid") }) + It("single-quotes comma-containing storage option values for PAX materialized views", func() { + // PAX minmax_columns/cluster_columns store a comma-separated column + // list as a single string value. Emitted unquoted, the embedded comma + // is read as an option separator and the restored matview DDL fails + // with `unrecognized parameter ""`. The value must be + // single-quoted so the WITH clause survives the round trip. + if !connectionPool.Version.IsCBDB() { + Skip("PAX access method is only available on Cloudberry") + } + testhelper.AssertQueryRuns(connectionPool, + "CREATE TABLE public.pax_mv_base(a int, b int, c int) DISTRIBUTED BY (a)") + defer testhelper.AssertQueryRuns(connectionPool, "DROP TABLE public.pax_mv_base CASCADE") + testhelper.AssertQueryRuns(connectionPool, + "CREATE MATERIALIZED VIEW public.pax_mv USING pax WITH (minmax_columns='a,b', cluster_columns='a,b', cluster_type='lexical') AS SELECT * FROM public.pax_mv_base DISTRIBUTED BY (a)") + defer testhelper.AssertQueryRuns(connectionPool, "DROP MATERIALIZED VIEW public.pax_mv") + + results := backup.GetAllViews(connectionPool) + var mv backup.View + for _, v := range results { + if v.Name == "pax_mv" { + mv = v + } + } + + // The comma-containing list values must be single-quoted ... + Expect(mv.Options).To(ContainSubstring("minmax_columns='a,b'")) + Expect(mv.Options).To(ContainSubstring("cluster_columns='a,b'")) + + // ... and the emitted WITH clause must recreate the matview verbatim. + // This asserts the wiring end to end: a failure to quote would make + // this CREATE MATERIALIZED VIEW fail. + testhelper.AssertQueryRuns(connectionPool, + fmt.Sprintf("CREATE MATERIALIZED VIEW public.pax_mv_restored USING pax%s AS SELECT * FROM public.pax_mv_base DISTRIBUTED BY (a)", mv.Options)) + testhelper.AssertQueryRuns(connectionPool, "DROP MATERIALIZED VIEW public.pax_mv_restored") + }) }) })