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

withStatement & withBind aren't quite useful #50

Open
ip1981 opened this issue Nov 26, 2016 · 5 comments
Open

withStatement & withBind aren't quite useful #50

ip1981 opened this issue Nov 26, 2016 · 5 comments
Assignees

Comments

@ip1981
Copy link

ip1981 commented Nov 26, 2016

withStatement & withBind aren't quite useful if you want to do a bunch of INSERTs.

I think I can use nextRow and discard the result, but this require specific type, e. g. IO (Maybe [Int]).

It would be great if Base.step were exported somehow through the Statement Base.Statement wrapper.

I could use Base.step, If I could get Base.Statement from Statement. But Statement constructor if not exported either.

What I mean:

submit :: SQLite.ToRow values => SQLite.Statement -> values -> IO ()
submit st v = SQLite.withBind st v $ void (SQLite.nextRow st :: IO (Maybe [Int]))
@nurpax
Copy link
Owner

nurpax commented Nov 30, 2016

I recently added executeMany which can be used for multi-inserts. Does that fit the bill or do you think we need a similar API to use with statements?

@ip1981
Copy link
Author

ip1981 commented Nov 30, 2016

Yeah... it's close, but not enough. In one of my use cases, number of rows is unknown: https://github.com/ip1981/sproxy2/blob/a3b5b7ac059e473e10749591c11aee97482b88d1/src/Sproxy/Server/DB.hs#L137-L139. It smells like conduit with PostgreSQL query as a producer :)

("Unknown" means it's impractical to fetch them all and then process one by one)

@nurpax
Copy link
Owner

nurpax commented Dec 27, 2016

Hi @ip1981 Sorry for taking so long to reply on this.

I wonder if it'd be enough to add a FromRow instance for (). So you could do

submit st v = SQLite.withBind st v $ void (SQLite.nextRow st :: IO (Maybe ()))

I think this would be fairly orthogonal to existing features. E.g., there's also a ToRow instance for () that converts the unit value to an empty list.

If we did this, the returned value from the statement would be error checked against an empty list. E.g., you wouldn't be able to use a unit nextRow on a SELECT statement.

Does this sound reasonable? I'll make a PR and send a review request.

@nurpax
Copy link
Owner

nurpax commented Dec 27, 2016

Actually, that didn't work out so well. nextRow always returns Nothing with inserts (since there are no rows to return).

Existing code:

nextRow :: (FromRow r) => Statement -> IO (Maybe r)
nextRow = nextRowWith fromRow

nextRowWith :: RowParser r -> Statement -> IO (Maybe r)
nextRowWith fromRow_ (Statement stmt) = do
  statRes <- Base.step stmt
  case statRes of
    Base.Row -> do
      rowRes <- Base.columns stmt
      let nCols = length rowRes
      row <- convertRow fromRow_ rowRes nCols
      return $ Just row
    Base.Done -> return Nothing

I guess I could either just expose step as is, or name it something like nextRow_ with an IO () return type.

@nurpax
Copy link
Owner

nurpax commented Dec 27, 2016

I played with the unit FromRow instance but it might be kind of a bad idea.

You can see the code here: https://github.com/nurpax/sqlite-simple/compare/unit-nextrow

With this feature, it still wouldn't be obvious how to use nextRow with INSERTs (you'd need come up with the idea to use FromRow () and/or read the docs quite a bit). I think a more explicit API for inserts would be better. I'm not sure I like my above nextRow_ proposal either.

@sigrlami sigrlami self-assigned this Feb 27, 2020
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

No branches or pull requests

3 participants