Skip to content
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

docs: don't recommend 'for all tables' publication for source-postgres #1026

Merged
merged 1 commit into from
May 4, 2023

Conversation

williamhbaker
Copy link
Member

@williamhbaker williamhbaker commented May 2, 2023

Description:

Doc updates for estuary/connectors#690. See that PR description for the technical background.

For manual setups these updated docs will instruct the user to create a targeted publication including only the tables they want to capture from rather than a "FOR ALL TABLES" publication which may have unwanted side effects.

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)

Notes for reviewers:

(anything that might help someone review this PR)


This change is Reviewable

Copy link
Member

@willdonnelly willdonnelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I think that it might still be worth noting somewhere on the page that the user can CREATE PUBLICATION ... FOR ALL TABLES in some circumstances, because adding tables individually can be a huge permissioning-related pain in some setups (AFAIK the query ALTER PUBLICATION ... ADD TABLE ... must be run either by a user with ownership of both the table and the publication, or by the database superuser, and transferring ownership after a table has been created is just a mess).

But on reflection I agree with Johnny's point about automatically breaking your database -- for the recommended sequences of copy-pastable setup commands here it's probably better to recommend the less dangerous approach, since either will work perfectly fine if you're doing this setup as the database superuser.

Copy link
Contributor

@oliviamiannone oliviamiannone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm from the writing side!

@williamhbaker williamhbaker merged commit c84092f into master May 4, 2023
@williamhbaker williamhbaker deleted the wb/postgres-not-all-tables branch May 4, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants