-
Notifications
You must be signed in to change notification settings - Fork 954
Feature/postgres cdc multi schema #4589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ness-david-dedu
wants to merge
24
commits into
redpanda-data:main
Choose a base branch
from
ness-david-dedu:feature/postgres_cdc_multi_schema
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
f903238
postgres_cdc: add multi-schema support
ness-david-dedu c38fd7a
postgres_cdc: reject empty quoted schema identifier and fix misleadin…
ness-david-dedu bda218d
postgres_cdc: add tests/current/ Docker Compose + Taskfile manual tes…
ness-david-dedu df2dca6
postgres_cdc: add commit_ts_ms and before metadata fields
ness-david-dedu e6bbd4c
Merge remote-tracking branch 'upstream/main' into feature/postgres_cd…
ness-david-dedu fcb0427
postgres_cdc: fix lint and docs
ness-david-dedu 7bcc3ac
postgres_cdc: review fixes and test coverage
ness-david-dedu d39de9a
postgres_cdc: fix tests
ness-david-dedu 5f9a2a8
Merge remote-tracking branch 'upstream/main' into feature/postgres_cd…
ness-david-dedu 0aa387a
test(cdctest): waive tigerbeetle_cdc conformance fields
ness-david-dedu ba7c384
postgres_cdc: fix lint
ness-david-dedu bd3e383
Merge remote-tracking branch 'upstream/main' into feature/postgres_cd…
ness-david-dedu caa0b1a
postgres_cdc: skip missing tables per-schema instead of failing whole…
ness-david-dedu c161947
postgres_cdc: fix lint
ness-david-dedu 0ed96fd
Update internal/impl/postgresql/pglogicalstream/schema_resolver.go
ness-david-dedu d893e1a
postgres_cdc: warn when schema pattern matches privilege-hidden schemas
ness-david-dedu 28b2710
Merge branch 'main' into feature/postgres_cdc_multi_schema
josephwoodward 0e662de
postgres_cdc: Address minor issues
josephwoodward e7dccc6
postgres_cdc: fix broken test
josephwoodward 80e2204
postgres_cdc: replace pg_schema with database_schema
josephwoodward fc960ce
postgres_cdc: move schema validation to unit test closer to use
josephwoodward 3411584
postgres_cdc: clean up redundant comment
josephwoodward bf28959
postgres_cdc: normalie test structure
josephwoodward 58d3be8
postgres_cdc: t.Context()
josephwoodward File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| // Copyright 2024 Redpanda Data, Inc. | ||
| // | ||
| // Licensed as a Redpanda Enterprise file under the Redpanda Community | ||
| // License (the "License"); you may not use this file except in compliance with | ||
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/redpanda-data/connect/v4/blob/main/licenses/rcl.md | ||
|
|
||
| package pgstream | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/redpanda-data/benthos/v4/public/service" | ||
|
|
||
| "github.com/redpanda-data/connect/v4/internal/license" | ||
| ) | ||
|
|
||
| // TestSchemaPatternValidation verifies that the schema field is validated | ||
| // during config parsing, before any network I/O is attempted. Success is | ||
| // asserted via newPgStreamInput returning no error - the constructor doesn't | ||
| // dial the database, so a valid pattern implies validation passed. | ||
| func TestSchemaPatternValidation(t *testing.T) { | ||
| tests := []struct { | ||
| pattern string | ||
| errContains string | ||
| }{ | ||
| {"public", ""}, | ||
| {"tenant_*", ""}, | ||
| {"*", ""}, | ||
| {`"MySchema"`, ""}, | ||
| // Regression test: len("") == 2 used to pass the old `len(s) < 2` guard. | ||
| // Fixed to `len(s) < 3`. | ||
| {`""`, "invalid quoted schema identifier"}, | ||
| {"1abc", "must start with a letter"}, | ||
| {`"unclosed`, "invalid quoted schema identifier"}, | ||
| {"schema-name", "invalid character"}, | ||
| {"", "schema cannot be empty"}, | ||
| {`"quoted*"`, "wildcard"}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.pattern, func(t *testing.T) { | ||
| // Single-quoted so the pattern (which may itself contain double | ||
| // quotes, e.g. `"MySchema"`) reaches validateSchemaPattern verbatim. | ||
| yaml := fmt.Sprintf(` | ||
| dsn: postgres://testuser:testpass@localhost:5432/testdb?sslmode=disable | ||
| schema: '%s' | ||
| slot_name: test_slot | ||
| tables: | ||
| - events | ||
| `, tt.pattern) | ||
|
|
||
| conf, err := newPostgresCDCConfig().ParseYAML(yaml, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| mgr := service.MockResources() | ||
| license.InjectTestService(mgr) | ||
|
|
||
| _, err = newPgStreamInput(conf, mgr) | ||
| if tt.errContains != "" { | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), tt.errContains) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
| }) | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression: unquoted schema names containing non-ASCII letters are now rejected at startup.
This validator only accepts
[a-zA-Z0-9_*], but the code path it replaces —sanitize.NormalizePostgresIdentifier, previously called onconfig.DBSchemainNewPgStream— accepts anyunicode.IsLetter/unicode.IsDigitrune (plus.): see sanitize.go#L443-L457.Failure scenario: an existing pipeline with
schema: münchen(a legal unquoted PostgreSQL identifier that previously normalised to"münchen"and worked) now fails config construction withinvalid schema: invalid character 'ü' at position 1 in schema pattern "münchen". The user has no way to know quoting is the workaround.Suggested fix: mirror
NormalizePostgresIdentifier's character classes here (unicode.IsLetter/unicode.IsDigit, plus_and*) so the pattern validator is a superset of what was previously accepted, rather than a stricter ASCII-only rule.