[talmacschen-arch]

Problem

GetTableStorage builds a table's WITH (...) clause by joining pg_class.reloptions with , :

array_to_string(reloptions, ', ') AS reloptions

A reloption is stored as a single name=value array element. The PAX storage options minmax_columns / cluster_columns hold a comma-separated column list as their value, so the value itself contains commas. Joining the array verbatim makes those embedded commas indistinguishable from the , separator between options, and the restored DDL

... USING pax WITH (minmax_columns=a,b, cluster_columns=a,b, cluster_type=lexical, compresstype=zstd)

fails:

ERROR: unrecognized parameter "b" (SQLSTATE 22023)

The table is then never created, so its data load, ALTER ... OWNER, indexes and ANALYZE all cascade into relation does not exist — the table and its data are lost on restore. Observed on Apache Cloudberry 2.1.0 restoring PAX tables with multi-column minmax_columns.

The PAX kernel stores these as a single string reloption and splits the value on commas itself (add_string_reloption + SplitDirectoriesString(value, ',')), so the value must reach the server single-quoted as one string.

Fix

Single-quote the value of any reloption whose value contains a comma. Options without a comma are emitted unchanged, so backup output for existing tables is unaffected. This mirrors the single-quoting already done in buildStorageOptionsString for storage servers.

Testing

Added an integration test in the GetTableStorageOptions block that creates a PAX table with multi-column minmax_columns/cluster_columns and asserts the emitted WITH clause (a) single-quotes the list values and (b) restores verbatim via a real CREATE TABLE. Gated to Cloudberry, where PAX is available.

[talmacschen-arch]

Cluster validation

Verified on a live Apache Cloudberry 2.1.0 / SynxDB4 (4.5.4-3) cluster with the patched gpbackup/gprestore.

A PAX table with multi-column list options was created and populated:

CREATE TABLE public.pmm(a int, b int, c int)
  USING pax WITH (minmax_columns='a,b', cluster_columns='a,b') DISTRIBUTED BY (a);
INSERT INTO public.pmm SELECT i, i%10, i%3 FROM generate_series(1,1000) i;

• Backup (patched): the emitted DDL carried the single-quoted list value — ... WITH (minmax_columns='a,b', ...).
• Restore into a fresh database succeeded, and the table came back with all its data: 1000 rows backed up → 1000 restored.

Kernel side (contrib/pax_storage/.../paxc_rel_options.cc) confirms why the quoting is required: minmax_columns/cluster_columns are registered via add_string_reloption and the value is split on , internally (SplitDirectoriesString(value, ',', ...)), so the comma-separated list must reach the server as one single-quoted string.

Without this fix, GetTableStorage emits the value unquoted (minmax_columns=a,b); the restored CREATE TABLE then fails with ERROR: unrecognized parameter "b" (observed in production as "ws_order_number" / "account_id"), the table is never created, and its data, owner, indexes and ANALYZE all cascade into relation does not exist — i.e. the table is lost on restore.

[liang8283]

Thanks for tracking this down — the root cause analysis is spot on, and the fix works for regular tables. I verified it end to end on a Cloudberry cluster with a full gpbackup → gprestore round trip, building both main and this branch:

Object	main	this PR
PAX table, minmax_columns='a,b', cluster_columns='a,b', ...	❌ unrecognized parameter "b", table + data lost	✅ all rows restored
PAX table, bloomfilter_columns='a,b', minmax_columns='b,c'	❌	✅
PAX partitioned table (parent + leaf partition)	❌	✅
PAX table, minmax_columns='A,B c' (mixed-case / space in names)	❌	✅
PAX table, minmax_columns='a' (single column)	✅	✅ unchanged output
PAX materialized view, minmax_columns='a,b'	❌	❌ still fails

The new integration test runs (not skipped) and passes on that cluster, and go test ./backup/... passes.

A few things I'd like to see addressed before merge:

1. Materialized views are not covered (blocking)

GetMaterializedViews builds its WITH clause the same way, in backup/queries_relations.go (both version6Query and atLeast7Query):

coalesce(' WITH (' || array_to_string(c.reloptions, ', ') || ')', '') AS options,

So a PAX materialized view still produces a broken WITH clause:

CREATE MATERIALIZED VIEW public.pax_mv USING pax WITH (minmax_columns=a,b) AS ...
ERROR: unrecognized parameter "b" (SQLSTATE 22023)

followed by ALTER MATERIALIZED VIEW ... OWNER failing with relation does not exist. Could the reloption formatting be pulled into one shared SQL snippet (e.g. a Go helper that returns the expression for a given column) and used in all three places, plus a matching integration test for the matview case?

2. Quoting only on , leaves closely related cases broken (suggestion)

Any value that is not a plain lowercase identifier/number has the same problem, even without a comma. Both of these fail on main and on this branch:

minmax_columns='A'    -> WITH (minmax_columns=A)    -> ERROR: invalid column name 'a'   (identifier gets downcased)
minmax_columns='x y'  -> WITH (minmax_columns=x y)  -> ERROR: syntax error at or near "y"

These aren't regressions from this PR, but since it's the same root cause it seems worth fixing together. Something like quoting whenever the value isn't a simple token keeps the output for existing tables byte-for-byte unchanged:

array_to_string(ARRAY(
	SELECT CASE WHEN substr(opt, strpos(opt, '=') + 1) ~ '^[a-z0-9_.]+$'
		THEN opt
		ELSE split_part(opt, '=', 1) || '=' || quote_literal(substr(opt, strpos(opt, '=') + 1))
	END
	FROM unnest(reloptions) WITH ORDINALITY AS u(opt, n)
	ORDER BY n), ', ')

I tried this on the same cluster against PAX (minmax_columns='a,b', cluster_columns='A', bloomfilter_columns='x y'), AO column (compresstype=zlib, compresslevel=1, blocksize=32768, checksum=true), heap (fillfactor=70, autovacuum_enabled=false) and a PAX matview. The generated WITH clauses all re-create cleanly, and the recreated relations' reloptions are identical to the originals. Unquoted options (compresstype=zstd, compresslevel=5, checksum=true, ...) are emitted exactly as before.

[talmacschen-arch]

Thanks for the thorough review and the cross-cluster verification!

Point 2 (non-token values) — fixed in this PR. I reproduced both cases on a live Cloudberry 2.1.0 (SynxDB4 4.5.4-3) cluster:

• minmax_columns='x y' (space) → ERROR: syntax error at or near "y" on restore;
• minmax_columns='A' on a quoted "A" column → emitted unquoted it silently downcases to column a, so the min/max index is built on the wrong column.

The quoting condition is now "quote unless the value is a simple token", i.e. it quotes whenever substr(value) !~ '^[a-z0-9_.]+$', which also subsumes the comma case. WITH ORDINALITY preserves option order. I confirmed existing heap/AO options (compresstype=zstd, compresslevel=5, fillfactor=70, checksum=true, ...) are emitted byte-for-byte identical to before, and that the new uppercase/spaced values restore verbatim (added an integration test for them).

Point 1 (materialized views) — deferred to a follow-up PR. I confirmed the matview defect is real (GetMaterializedViews emits WITH (minmax_columns=a,b) → unrecognized parameter "b"). To keep this PR focused on base tables I've left GetMaterializedViews (queries_relations.go) for a separate PR and added a note to that effect in the commit message. I'll open it with a shared formatting helper and a matview integration test as you suggested.

[talmacschen-arch]

Follow-up on point 1 (materialized views): the materialized-view path is now fixed in a dedicated PR — #130 ("Quote non-token storage option values for materialized views"). It applies the same quoting to GetAllViews (both the GPDB6 and GPDB7+/Cloudberry queries) via a shared helper, so a PAX matview with minmax_columns/cluster_columns no longer fails restore with unrecognized parameter. Verified end to end on a live Cloudberry 2.1.0 cluster.

For completeness, incremental materialized views (IVM) are handled in #131, which builds on the same GetAllViews query.

This PR (#127) stays scoped to base tables as discussed.