jw/pg multi schema - #4692
Conversation
josephwoodward
commented
Aug 12, 2026
- postgres_cdc: add multi-schema support
- postgres_cdc: reject empty quoted schema identifier and fix misleading godoc
- postgres_cdc: add tests/current/ Docker Compose + Taskfile manual test harness
- ** postgres_cdc: add commit_ts_ms and before metadata fields**
- postgres_cdc: fix lint and docs
- postgres_cdc: review fixes and test coverage
- postgres_cdc: fix tests
- test(cdctest): waive tigerbeetle_cdc conformance fields
- postgres_cdc: fix lint
- postgres_cdc: skip missing tables per-schema instead of failing whole publication
- postgres_cdc: fix lint
- Update internal/impl/postgresql/pglogicalstream/schema_resolver.go
- postgres_cdc: warn when schema pattern matches privilege-hidden schemas
- postgres_cdc: Address minor issues
- postgres_cdc: fix broken test
- postgres_cdc: replace pg_schema with database_schema
- postgres_cdc: move schema validation to unit test closer to use
- postgres_cdc: clean up redundant comment
- postgres_cdc: normalie test structure
- postgres_cdc: t.Context()
- postgres_cdc: revert to existing behaviour
- postgres_cdc: improve on missing table coverage
- update docs
- postgres_cdc: add schema_pattern field for multi-schema replication
- postgres_cdc: fix unicode schema_pattern rejection, sync changelog
- postgres_cdc: fix stale schema_pattern refs in manual test harness
- postgres_cdc: add excluded_schema
- postgres_cdc: fix failing signalling tests
- postgres_cdc: add further test coverage for uuid schemas
…t harness Two-schema (tenant_a, tenant_b) Postgres 16 setup that exercises the multi-schema CDC pipeline end-to-end. Also adds schema_validation unit tests that verify invalid patterns are rejected at startup without a DB.
…c_multi_schema # Conflicts: # CHANGELOG.md
Co-authored-by: Joseph Woodward <joseph.woodward@xeuse.com>
information_schema.schemata only lists schemas the connecting role can see, so a schema hidden by missing USAGE was silently dropped from a matched pattern with no signal to the user. resolveSchemas now cross-checks pg_catalog.pg_namespace, which isn't privilege-filtered, and reports those as inaccessible so logical_stream.go can warn instead of skipping silently.
If no schema is found and tables is empty then we want to ensure we create publication for all tables.
Introduces schema_pattern as a new optional field for glob-based multi-schema CDC, instead of overloading schema with pattern semantics. This keeps schema backwards compatible: it reverts to exact-name-only behavior (now defaulting to "public"), while schema_pattern opts into the glob resolution path. Setting both is a config error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
validateSchemaPattern only accepted [a-zA-Z0-9_*], stricter than sanitize.NormalizePostgresIdentifier (unicode.IsLetter/IsDigit) which governs the exact-name schema path. Since schema_pattern is now the sole caller of validateSchemaPattern, an unquoted pattern with non-ASCII letters (e.g. münchen) was rejected at startup even though the equivalent schema value has always been accepted. Widened the character classes to match, using utf8.DecodeRuneInString for the first-character check instead of a raw byte cast. Also corrected the Unreleased changelog entry, which still described the old design where schema itself accepted glob patterns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
internal/impl/postgresql/tests/current/ is a docker-compose-backed Taskfile harness for manually exercising multi-schema CDC against a real Postgres (tenant_a/tenant_b). It predates the schema/schema_pattern split and still used schema: tenant_* directly, which the split turns into a literal (and invalid) exact schema name instead of a glob. - test_config.yaml: schema -> schema_pattern for the tenant_* glob. - Taskfile.yaml test:invalid-schema: schema="" is now a no-op (unset schema_pattern falls back to schema's "public" default), so it no longer exercises a failure path. Repointed at schema_pattern=1abc, which still fails fast in newPgStreamInput before any DB connection, matching the task's original intent. Verified by bringing the compose stack up and running the smoke test; config loads and reaches runtime init (blocked only by the license check in this sandboxed environment, which is unrelated). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| fieldSchemaInclude = "schema_include" | ||
| fieldSchemaExclude = "schema_exclude" |
There was a problem hiding this comment.
CDC config naming: schema_include/schema_exclude are bespoke names where the fleet already has canonical include/exclude
CONTRIBUTING §5.3.1 lists the canonical CDC filtering fields as "tables (or include/exclude)", and §5.3.3 says "Do not invent bespoke names where a canonical one already exists… New connectors must use the canonical names from the start." §5.5.7 names oracledb_cdc as the reference for include/exclude matching.
The reference implementation uses plain include/exclude holding regexes over schema-qualified names, which already covers schema selection:
connect/internal/impl/oracledb/input_oracledb_cdc.go
Lines 214 to 221 in 96b8d78
ociFieldTablesExclude = "exclude"
ociFieldTablesInclude = "include"
// Description("Regular expressions for tables to include.").Example("SCHEMA.PRODUCTS")This PR instead introduces schema_include (a single string) and schema_exclude (a list) with glob semantics rather than regex, so postgres_cdc ends up with a third filtering vocabulary alongside tables and oracle's include/exclude. That is exactly the divergence §5.3.3 and §3.1.5 ("codebase feels consistent with other Redpanda Connect connectors") are meant to prevent, and it will be a breaking rename later when postgres_cdc converges.
Suggested fix: express schema filtering through the canonical include/exclude regex fields matched against schema.table, as oracledb_cdc does. If glob-over-schemas is deliberately preferred here, please get the deviation agreed and record it in the conformance gate at internal/plugins/cdctest so the fleet decision is explicit rather than implicit.
| fieldSchemaInclude = "schema_include" | ||
| fieldSchemaExclude = "schema_exclude" |
There was a problem hiding this comment.
Bespoke include/exclude naming and matching syntax diverges from the CDC fleet (CONTRIBUTING §5.3.3, §5.5.7, §3.1.5)
CONTRIBUTING.md §5.3.1 names include/exclude as the canonical selection fields, and §5.3.3 says "Do not invent bespoke names where a canonical one already exists". §5.5.7 specifies "Include/exclude regex table matching. Reference: oracledb_cdc."
The fleet already implements this axis as top-level include/exclude string lists of regular expressions matched against SCHEMA.TABLE:
- (
connect/internal/impl/oracledb/input_oracledb_cdc.go
Lines 40 to 48 in bc81de9
exclude/include) - (
connect/internal/impl/oracledb/input_oracledb_cdc.go
Lines 213 to 222 in bc81de9
Regular expressions for tables to include./... exclude., e.g.SCHEMA.PRODUCTS) internal/impl/mssqlserver/input_mssqlserver_cdc.gouses the sameinclude/excludenames.
This PR instead adds schema_include (single string) and schema_exclude (string list) with a glob (*) syntax that is neither the canonical name nor the fleet's regex semantics, so a user configuring multi-schema capture writes something different in postgres_cdc than in oracledb_cdc/microsoft_sql_server_cdc. §3.1.5 asks that "the codebase feels consistent with other Redpanda Connect connectors, avoiding bespoke or idiosyncratic implementations."
Suggested fix: express schema selection through the fleet's include/exclude regex fields matched against schema.table (which subsumes both the schema glob and the per-schema tables resolution added here), or — if a schema-scoped field is genuinely required — align at least the matching syntax with the fleet's regexes and get the new names recorded in the §5.3 canonical list / internal/plugins/cdctest before landing.
| } | ||
| return nil, fmt.Errorf("no schemas found matching schema_include pattern %q", config.DBSchemaInclude) | ||
| } | ||
| config.Logger.Infof("schema_include pattern %q resolved to %d schema(s): %v", config.DBSchemaInclude, len(schemas), schemas) |
There was a problem hiding this comment.
Schema-set drift across reconnects is silent (CONTRIBUTING §1.2.2, §3.1.4)
NewPgStream re-resolves schema_include on every connect and reconnect, but nothing compares the newly resolved set against the previous one, so two materially different outcomes are logged only as the ordinary Infof on this line. The field description added in this PR spells both out:
connect/internal/impl/postgresql/input_pg_stream.go
Lines 204 to 207 in bc81de9
- A schema that starts matching after the slot exists is added to the publication, but its pre-existing rows are never snapshotted — the docs say they "are silently missing from the output".
- A schema that is dropped/renamed has its tables removed from the publication by
CreatePublicationa few lines below () — the docs say "nothing is ever logged about the resulting publication drop itself".connect/internal/impl/postgresql/pglogicalstream/pglogrepl.go
Lines 360 to 378 in bc81de9
CONTRIBUTING §1.2.2 requires that "Unexpected behavior should emit warning or error logs", and §3.1.4 asks for an implementation with "no known bugs or missing core functionality". Silently ceasing to replicate a tenant, or silently emitting an un-snapshotted tenant under stream_snapshot: true, are exactly the cases an operator needs a signal for — documenting them under §1.2.3 doesn't remove the §1.2.2 obligation.
Suggested fix: persist the resolved schema set on the Stream/config and, on each subsequent resolution, Warnf the added schemas (noting their snapshot is skipped) and the removed schemas (noting their tables are being dropped from the publication). The same applies to the inaccessibleSchemas warning above, which currently fires identically on every reconnect regardless of whether the set changed.
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/redpanda-data/connect/blob/main/licenses/rcl.md | ||
| // https://github.com/redpanda-data/connect/v4/blob/main/licenses/rcl.md |
There was a problem hiding this comment.
License header URL was changed to a broken link. This line previously read https://github.com/redpanda-data/connect/blob/main/licenses/rcl.md and this PR rewrote it to insert /v4/, which is not a real GitHub path (there is no redpanda-data/connect/v4 repository) — it 404s.
The canonical RCL header template is https://github.com/redpanda-data/connect/blob/bc81de9be3fb2c566b8998176479671610c240ea/licenses/rcl_header.go.txt, and the godev reference in CLAUDE.md documents the same non-/v4/ URL; 300+ files in the repo use it.
Please revert this line to the template URL, and use the template URL in the three new files added by this PR (pglogicalstream/schema_resolver.go, schema_resolver_test.go, schema_resolver_integration_test.go) rather than propagating the /v4/ variant. Note the new files are also inconsistent with each other on the copyright year (2026 vs 2024).
| schemas = remaining | ||
| } | ||
|
|
||
| if len(inaccessibleSchemas) > 0 && !slices.Equal(inaccessibleSchemas, cfg.previouslyInaccessibleSchemas) { |
There was a problem hiding this comment.
Order-sensitive comparison of an unordered query result → repeated warnings on every reconnect.
resolveSchemas builds inaccessibleSchemas from SELECT nspname FROM pg_catalog.pg_namespace WHERE ... with no ORDER BY (schema_resolver.go#L107-L131), so row order is not guaranteed to be stable between connects. slices.Equal here is therefore an order-sensitive comparison of what is semantically a set: if PostgreSQL returns the same two hidden schemas in a different order after a reconnect, the "missing USAGE privilege" warning re-fires even though nothing changed.
This is inconsistent with the adjacent drift detection a few lines below, which correctly uses set semantics via diffSchemaSets (schema_resolver.go#L186-L194).
Suggested fix: compare as a set (e.g. reuse diffSchemaSets and warn only when added/removed is non-empty), or add a deterministic ORDER BY to both queries in resolveSchemas so the slice comparison is meaningful. Repeated spurious warnings on a healthy pipeline also cut against CONTRIBUTING.md §1.2.2 ("Normal operation should emit no logs").
| // conditionalConnectors lists CDC inputs that are only registered under specific | ||
| // build tags (e.g. cgo). They are exempt from the stale-entry guard when not | ||
| // registered, but are still subject to conformance checks when they are. | ||
| var conditionalConnectors = map[string]bool{ |
There was a problem hiding this comment.
Out of scope for this PR.
This PR is scoped to postgres_cdc multi-schema replication (schema_include/schema_exclude), but this hunk adds a conditionalConnectors build-tag exemption and rewrites all five tigerbeetle_cdc waiver reasons (cdc_conformance_test.go#L102-L108) — none of which relate to postgres_cdc. The new postgres_cdc fields aren't part of canonicalFields, so this test needs no change for this feature.
CONTRIBUTING.md §3.1.1: "the PR stays within that agreed scope; additional components or capabilities beyond it are proposed and reviewed separately."
Suggested fix: split the cgo-conditional exemption and the tigerbeetle waiver re-classification into their own PR, so the CDC conformance-gate change gets reviewed on its own merits rather than as a drive-by in a Postgres feature.
| @@ -0,0 +1,120 @@ | |||
| version: "3" | |||
There was a problem hiding this comment.
Bespoke manual harness committed at a non-descriptive path, with no README, duplicating coverage this PR already adds in CI.
internal/impl/postgresql/tests/current/ is a new top-level convention for this repo. The established equivalent is bench/, which this connector already has and which ships a README explaining how to run it (bench/README.md, bench/Taskfile.yaml). "current" doesn't describe what the directory holds, and nothing here tells a future reader when to reach for it.
Everything it exercises — the tenant_* glob, schema_exclude: [tenant_c] carve-out, database_schema metadata, and the two negative-validation cases — is already covered by testcontainers-backed tests added in this same PR (TestIntegrationMultiSchemaExcludeCarvesOutTenant, TestIntegrationMultiSchemaIncludeExcludeConfigValidation) plus the unit test TestSchemaExcludeRequiresSchemaInclude. Those run in CI; this harness can only silently rot.
CONTRIBUTING.md §3.1.5 ("codebase feels consistent with other Redpanda Connect connectors, avoiding bespoke or idiosyncratic implementations") and §6 ("No binaries, build artifacts, or local tooling committed").
Suggested fix: drop the directory, or — if the manual smoke path is genuinely wanted — move it under the existing bench/-style layout with a README stating its purpose and its relationship to the CI integration tests.
e219a43 to
ad1c132
Compare
| } | ||
| return nil, fmt.Errorf("no schemas found matching schema_include pattern %q", cfg.DBSchemaInclude) | ||
| } | ||
| cfg.Logger.Infof("schema_include pattern %q resolved to %d schema(s): %v", cfg.DBSchemaInclude, len(schemas), schemas) |
There was a problem hiding this comment.
Info logs fire on every connect and reconnect (CONTRIBUTING §1.2.2: "Provides relevant logging to support troubleshooting. Unexpected behavior should emit warning or error logs. Normal operation should emit no logs.")
Three new Infof calls run on the success path of every schema resolution, and — as the schema_include field docs added in this PR state — resolution is re-run on every connect and reconnect, including the automatic reconnects after a transient replication failure:
schema_resolver.go:146—schema_exclude %v excluded %d schema(s) ...schema_resolver.go:162—schema_include pattern %q resolved to %d schema(s): %v(this line)logical_stream.go:156—%q has notableslist configured: auto-discovered ...
None of these describe unexpected behaviour, so on a flapping connection they become steady-state noise. This is also inconsistent with the adjacent inaccessible-schema warning at schema_resolver.go:151-154, which was deliberately de-duplicated across reconnects via previouslyInaccessibleSchemas.
Suggested fix: drop these to Debugf, or gate them on the resolved set actually having changed since the previous connect (the previouslyResolvedSchemas bookkeeping right below already computes exactly that).
| // diffSchemaSets reports schemas present in current but not previous | ||
| // (added) and vice versa (removed). Callers should ignore the result when | ||
| // previous is nil - that's the first resolution, not real drift. | ||
| func diffSchemaSets(previous, current []string) (added, removed []string) { |
There was a problem hiding this comment.
New reconnect-drift logic has no test coverage.
Every other new helper in this file got a unit test in schema_resolver_test.go (globToLike, escapeLike, schemaPatternToLike, schemaMatchesExcludePattern), but diffSchemaSets and the previouslyResolvedSchemas / previouslyInaccessibleSchemas bookkeeping it feeds (schema_resolver.go:151-173) are untested — no unit test and no integration test exercises a second Connect() against a changed schema set.
This is the behaviour the schema_include docs call out as "easy to miss" (a schema appearing between reconnects is never snapshotted; a schema disappearing is silently dropped from the publication), so it's the part most worth pinning down. diffSchemaSets is a pure function over two slices and is trivially table-testable; the added/removed warning paths can be driven by resolving twice against a DB where a schema is created/dropped in between.
Per the project test patterns, changed code should carry tests.
| if cfg.previouslyResolvedSchemas != nil { | ||
| added, removed := diffSchemaSets(cfg.previouslyResolvedSchemas, schemas) | ||
| if len(added) > 0 { | ||
| cfg.Logger.Warnf("schema_include pattern %q now also matches schema(s) %v that did not match on the previous connect; their tables are being added to the publication, but any rows already in them will NOT be snapshotted even if stream_snapshot is enabled - only changes made from now on will be captured", cfg.DBSchemaInclude, added) |
There was a problem hiding this comment.
Silent data loss for schemas matched after the slot exists.
This warning states the defect in its own text: a schema that starts matching schema_include after the replication slot has been created gets its tables added to the publication, but "any rows already in them will NOT be snapshotted even if stream_snapshot is enabled". Because postgres_cdc uses the replication slot itself as its checkpoint, this also applies across a full process restart, not just a reconnect — once the slot exists, NewPgStream never re-runs the snapshot for the newly matched schema.
This is the primary use case for the feature: in a multi-tenant, schema-per-tenant database, provisioning a new tenant schema is the normal event, and every row written to that schema before the next reconnect is dropped from the output with only a WARN log.
This conflicts with:
- CONTRIBUTING.md §5.5.2 — "Snapshot + streaming with a gap-free handoff".
- CONTRIBUTING.md §3.1.4 — "The implementation is complete and correct, with no known bugs or missing core functionality." Documenting a known silent-loss path does not satisfy this.
Suggested direction: when diffSchemaSets reports added schemas and stream_snapshot is enabled, snapshot the newly discovered schemas before resuming streaming (capturing their stream position before the snapshot read, per §5.5.2), rather than adding them to the publication caught-up. If that is out of scope for this PR, the safer default would be to fail the reconnect instead of silently proceeding, with an explicit opt-in field for the current lossy behaviour.
| fieldSchemaInclude = "schema_include" | ||
| fieldSchemaExclude = "schema_exclude" |
There was a problem hiding this comment.
Bespoke include/exclude naming and pattern syntax vs. the CDC fleet.
The fleet already has a canonical include/exclude filter for CDC inputs. CONTRIBUTING.md §5.3.1 lists the canonical names as tables` (or `include`/`exclude`), §5.3.3 says "Do not invent bespoke names where a canonical one already exists", and §5.5.7 names the reference implementation: "Include/exclude regex table matching. Reference: oracledb_cdc."
oracledb_cdc implements that as top-level include/exclude string lists of regular expressions matched against SCHEMA.TABLE:
connect/internal/impl/oracledb/input_oracledb_cdc.go
Lines 214 to 222 in 0a86a33
This PR instead adds schema_include (a single string) and schema_exclude (a list), both using a *-glob syntax rather than regex, for a filtering job the canonical include/exclude regex already covers (tenant_.*\..*). A user moving between oracledb_cdc and postgres_cdc now meets two different field names and two different pattern languages for the same concept, which is what §5.3.3 and §3.1.5 ("feels consistent with other Redpanda Connect connectors, avoiding bespoke or idiosyncratic implementations") are meant to prevent.
Worth reconciling with the CDC standard before this lands — either adopt include/exclude with regex semantics, or record an explicit, reasoned divergence.
| // diffSchemaSets reports schemas present in current but not previous | ||
| // (added) and vice versa (removed). Callers should ignore the result when | ||
| // previous is nil - that's the first resolution, not real drift. | ||
| func diffSchemaSets(previous, current []string) (added, removed []string) { |
There was a problem hiding this comment.
diffSchemaSets has no unit test.
Every other pure helper added in this file — globToLike, escapeLike, schemaPatternToLike, schemaMatchesExcludePattern — got a table-driven unit test in the new schema_resolver_test.go, but diffSchemaSets did not. It is the function that decides whether the reconnect drift warnings fire, including the silent-data-loss warning at schema_resolver.go#L167, and none of the new integration tests exercise a reconnect with a changed schema set — so this logic is currently untested end to end as well.
It is a pure function over two slices; a table test covering added-only, removed-only, both, no change, and a nil previous costs very little and matches the pattern of its siblings in the same file. Worth adding per CONTRIBUTING.md §1.3.2.
| @@ -0,0 +1,120 @@ | |||
| version: "3" | |||
There was a problem hiding this comment.
tests/current/ duplicates the automated integration coverage and doesn't match any existing directory convention.
This adds a manually-driven docker-compose + Taskfile harness (Taskfile.yaml, docker-compose.yaml, setup.sql, test_config.yaml, test_config_schema_exclude_no_include.yaml) that exercises exactly what the new testcontainers-backed tests in this same PR already cover automatically — tenant_a/tenant_b/tenant_c with schema_include: tenant_* and schema_exclude: tenant_c is TestIntegrationMultiSchemaExcludeCarvesOutTenant, and test:schema-exclude-requires-include is TestSchemaExcludeRequiresSchemaInclude.
Harnesses of this shape do exist in the repo, but always under a named purpose directory — internal/impl/postgresql/bench/, internal/impl/iceberg/demo/, internal/impl/iceberg/e2e/, internal/impl/aws/resources/. There is no tests/ directory under internal/impl/ anywhere else, and current/ doesn't convey what it holds or when it stops being "current". postgresql/ already has a bench/ tree that this could fold into if a real-endpoint harness is the intent (which would also serve §1.3.4).
Per §3.1.5 ("avoiding bespoke or idiosyncratic implementations") and the §6 checklist ("No binaries, build artifacts, or local tooling committed"): either drop this in favour of the integration tests, or move it under a conventional directory name.
| - | | ||
| set +e | ||
| go run ../../../../../cmd/redpanda-connect/main.go run \ | ||
| --set 'input.postgres_cdc.schema_include=1abc' \ |
There was a problem hiding this comment.
test:invalid-schema no longer exercises the failure path it documents.
The task feeds schema_include=1abc and the trailing comment asserts an invalid schema_include error with a non-zero exit — but validateSchemaPattern deliberately accepts a leading digit. The unit test in this same PR pins that behaviour explicitly:
{"1abc", ""},— "a leading digit is not an identifier-syntax violation here … so1abcmust be as valid as any other unquoted pattern"
https://github.com/redpanda-data/connect/blob/bd0346b610cbc5cce5e0318be6b669b5af03d501/internal/impl/postgresql/input_pg_stream_test.go#L451-L458
So this task now starts a real pipeline run against PG_DSN instead of failing fast. After f39ff34 allowed leading digits, the only unquoted pattern validateSchemaPattern still rejects is one containing a " (e.g. a"b) — point the task at that, or drop it, since TestSchemaIncludeValidation already covers this deterministically without Docker.
Secondly, the exit-code check is inert in both smoke tasks: go run … 2>&1 | head -5 followed by echo "exit $?" reports head's status, not the binary's, so it prints exit 0 regardless of whether the connector failed. Capture the status with PIPESTATUS[0] or drop the head pipe.
Finally, on placement: internal/impl/postgresql/tests/ is the only tests/ directory under internal/impl/. Compose-backed harnesses in this repo live under bench/, demo/, or resources/ — and this connector already has internal/impl/postgresql/bench/. CONTRIBUTING §3.1.5 asks that connectors feel consistent with the rest of the codebase rather than introducing bespoke layouts; current/ in particular doesn't convey what the harness is for.
| Optional(). | ||
| Default("public"), | ||
| ). | ||
| Field(service.NewStringField(fieldSchemaInclude). |
There was a problem hiding this comment.
CDC standard conformance: schema_include / schema_exclude are bespoke names with bespoke matching semantics for a selection axis the fleet already has canonical fields for.
CONTRIBUTING §5.3.1 lists the canonical config names as ``tables(orinclude`/`exclude`)`, §5.3.3 says "Do not invent bespoke names where a canonical one already exists", and §5.5.7 specifies "Include/exclude regex table matching. Reference: `oracledb_cdc`". The reference implementation exposes those as top-level regex lists over fully-qualified `SCHEMA.TABLE` names:
connect/internal/impl/oracledb/input_oracledb_cdc.go
Lines 213 to 222 in bd0346b
microsoft_sql_server_cdc uses the same include/exclude pair. That shape already covers the multi-tenant case this PR targets (^tenant_.*\..*$ selects every table in every tenant_* schema, and exclusions are the same list with the opposite sense), so a user moving between postgres_cdc and oracledb_cdc now meets two different names and two different pattern languages — glob * here vs. regex there, which §5.5.7 names explicitly.
Worth confirming this divergence was agreed under §3.1.1 before it lands, since renaming later is a breaking change (§5.3.2 only makes it non-breaking if the canonical field is the one added). If glob-over-schemas really is the right UX for Postgres, please say why in the PR description and add a note to internal/plugins/cdctest so the deviation is recorded rather than discovered later.