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

ext/pdo_pgsql: Expanding COPY input from an array to an iterable #15893

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

KentarouTakeda
Copy link
Contributor

@KentarouTakeda KentarouTakeda commented Sep 15, 2024

This pull request allows PDO::pgsqlCopyFromArray to accept Iterable instead of just array. This is not a breaking change since it still accepts array.

The PostgreSQL COPY statement is a great interface. We can bulk insert a set of records by inputting them as CSV or TSV. PostgreSQL also supports processing them as a stream.

However, the current pgsqlCopyFromArray does not take full advantage of this capability. It requires that the records to be inserted be input as a single array, so it cannot be processed by piping external input to PostgreSQL's COPY.

This weakness will be overcome by supporting not only simple arrays but also Traversable and Generator.

@devnexen
Copy link
Member

devnexen commented Sep 15, 2024

Nice feature and qualifies for the next major release I think. looking ok overall, I would suggest trying to refactor, as much as you can, common code between the two modes.

@KentarouTakeda
Copy link
Contributor Author

I'll try it.

@KentarouTakeda KentarouTakeda marked this pull request as draft September 15, 2024 09:04
@KentarouTakeda
Copy link
Contributor Author

I tried to do as much as I could.
(Actually, I'm not very familiar with the C language. Please feel free to let me know if I'm doing something strange.)

@KentarouTakeda KentarouTakeda marked this pull request as ready for review September 15, 2024 10:58
@devnexen
Copy link
Member

Couple of things to look at I think, good news is because of this

qualifies for the next major release I think

you have the time.

see next comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants