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

Add foldCopyData helper function #56

Merged
merged 1 commit into from
Nov 25, 2020
Merged

Add foldCopyData helper function #56

merged 1 commit into from
Nov 25, 2020

Conversation

sestrella
Copy link
Contributor

@sestrella sestrella commented Nov 23, 2020

This PR introduces a new function called foldCopyData which folds over COPY TO STDOUT query.

Copy link
Collaborator

@phadej phadej left a comment

Choose a reason for hiding this comment

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

The withCopyData and getAllCopyData are too specific functions.

I'd rather write something like

-- | Fold over @COPY TO STDOUT@ query. ... same notes as for 'getCopyData'.
foldCopyData
    :: (a -> ByteString -> IO a)  -- ^ Accumulate one row of the result
    -> (a -> Int64 -> IO b)       -- ^ Post-process accumulator with a count of rows
    -> a                          -- ^ Initial accumulator
    -> IO b                       -- ^ Result

and let people specialise.

Your getAllCopyData has performance issue, ++ is quadratic when used like that.
Check difference lists (i.e. composing [a] -> [a] functions).
It would be a simple instantiation of foldCopyData

@sestrella
Copy link
Contributor Author

sestrella commented Nov 24, 2020

@phadej thank you for the feedback! I completely agree with you, I think both functions could be replaced by a more general function. I'll work on it.

@sestrella sestrella changed the title Add getCopyData helper functions to consume all rows Add foldCopyData helper function Nov 25, 2020
Copy link
Collaborator

@phadej phadej left a comment

Choose a reason for hiding this comment

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

Looks better!

src/Database/PostgreSQL/Simple/Copy.hs Outdated Show resolved Hide resolved
src/Database/PostgreSQL/Simple/Copy.hs Outdated Show resolved Hide resolved
@phadej phadej merged commit 227fcb3 into haskellari:master Nov 25, 2020
@phadej
Copy link
Collaborator

phadej commented Nov 25, 2020

Good, thanks!

@sestrella sestrella deleted the all_copy_data branch November 25, 2020 15:59
@phadej
Copy link
Collaborator

phadej commented Jan 6, 2021

Released in postgresql-0.6.4

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.

2 participants