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

Use copy with binary. #857

Merged
merged 8 commits into from
Aug 6, 2024
Merged

Conversation

VaibhaveS
Copy link
Contributor

@VaibhaveS VaibhaveS commented Jul 31, 2024

  • The binary format is a little faster than the text format (https://www.postgresql.org/docs/current/sql-copy.html).
  • It's also useful in cases where the data stored in the BYTEA column, wherein the size in text format may exceed the 1GB field limit size of PostgreSQL.
pgsql.c:3159              [SOURCE 836084] [XX000] ERROR:  invalid memory alloc request size 1119193891
pgsql.c:3174              [SOURCE 836084] Context: Failed to fetch data from source
pgsql.c:3159              [TARGET 5216] [57014] ERROR:  COPY from stdin failed: Failed to get data from source

@dimitri
Copy link
Owner

dimitri commented Jul 31, 2024

While I like the idea, the main difficulty with it is how to make sure that the source and target Postgres servers are binary compatible? It's very hard to say because it is not well documented; and Postgres server compile time options will have an impact in that area (e.g. timestamp internal representation can be either bigint or float8).

Is is possible to use the binary protocol for selected columns, as in the libpq API? Then we could use it only for bytea columns, or a list of data type we know won't be a problem.

Otherwise we can add a new command line option such as --use-copy-binary to drive the decision, leaving the compatibility check to the users.

@VaibhaveS
Copy link
Contributor Author

VaibhaveS commented Jul 31, 2024

Is is possible to use the binary protocol for selected columns, as in the libpq API? Then we could use it only for bytea columns, or a list of data type we know won't be a problem.

I'll check this out.

EDIT: There isn't.

@VaibhaveS VaibhaveS force-pushed the use-copy-with-binary branch 2 times, most recently from 3cd2c07 to 9216022 Compare July 31, 2024 14:17
@VaibhaveS
Copy link
Contributor Author

The actions are failing due to a weird reason.

@dimitri
Copy link
Owner

dimitri commented Aug 2, 2024

The actions are failing due to a weird reason.

We are having a look with @hanefi ; the current suspicion is that the old spelling docker-compose is not available anymore, the new spelling docker compose might need to be used now. I remember trying and failing to implement that transition some time ago, where it was the opposite problem. If that hypotheses explains it, then we should have a fix shortly (enough).

@dimitri
Copy link
Owner

dimitri commented Aug 2, 2024

If you rebase to the current main branch, it should be all good again now.

@dimitri dimitri requested a review from hanefi August 2, 2024 15:25
@dimitri dimitri added the enhancement New feature or request label Aug 2, 2024
@dimitri dimitri added this to the v0.17 milestone Aug 2, 2024
Copy link
Contributor

@hanefi hanefi left a comment

Choose a reason for hiding this comment

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

One minor comment about whitespaces, otherwise looks good.

src/bin/pgcopydb/cli_clone_follow.c Outdated Show resolved Hide resolved
@VaibhaveS VaibhaveS requested a review from hanefi August 6, 2024 09:47
@dimitri
Copy link
Owner

dimitri commented Aug 6, 2024

Was about to merge then realized we are missing a long description of the new option in the docs at that place: https://pgcopydb.readthedocs.io/en/latest/ref/pgcopydb_clone.html#options -- @VaibhaveS can you add that?

Copy link
Owner

@dimitri dimitri left a comment

Choose a reason for hiding this comment

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

Missing option coverage in docs.

@dimitri dimitri merged commit d0a15d8 into dimitri:main Aug 6, 2024
19 checks passed
@VaibhaveS VaibhaveS deleted the use-copy-with-binary branch August 6, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants