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

Purpose of return type of Stream in copyIn #2

Open
ivan-m opened this issue Jun 8, 2017 · 3 comments
Open

Purpose of return type of Stream in copyIn #2

ivan-m opened this issue Jun 8, 2017 · 3 comments
Labels

Comments

@ivan-m
Copy link

ivan-m commented Jun 8, 2017

The copyIn function takes a Stream with a return type of Maybe ByteString, which seems to be used to determine if the copying was successful.

Is this meant to be an upstream/source indicator of success as an error message rather than actual data that should have been in the Stream?

@ocharles
Copy link
Collaborator

The ByteString returned is a textual error message that will be presented to the user if the stream can no longer produce all the data required for the copy. For example, maybe the stream is doing some parsing, and it encounters an invalid record - in this case it can terminate the stream early, and return Just "Invalid record". PostgreSQL will terminate the copy entirely (no partial data will be added), and the exception will be added to the PostgreSQL logs.

Does this help? I'm open to suggestions on better documentation for this function.

@ivan-m
Copy link
Author

ivan-m commented Jun 15, 2017

I think it was just the "If the stream terminates with 'Just' @error@, the error message will be logged." wording that confused me; maybe something like:

The copy is successful with 'Nothing' result; otherwise, the error (which is also logged on the server) is returned.

Also, Maybe is usually used to indicate a successful result; it may be worth defining data Result a = Err a | Success (which I've done in the past).

@ocharles
Copy link
Collaborator

Both great points, and I think this library is new enough that I can afford to break the API in such a way. It's certainly confusing that Just actually signifies failure.

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

No branches or pull requests

2 participants