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

sql: propagate the number of rows updated / inserted to the command-line client #3993

Closed
knz opened this issue Jan 27, 2016 · 6 comments
Closed
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Milestone

Comments

@knz
Copy link
Contributor

knz commented Jan 27, 2016

The client should report the number of rows altered with insert/update. This applies both to the interactive client and "-e".

@knz knz self-assigned this Jan 27, 2016
@knz
Copy link
Contributor Author

knz commented Jan 29, 2016

Useful to report success/failure of CAS operations in #4036

@petermattis petermattis changed the title Propagate the number of rows updated / inserted to the command-line client sql: propagate the number of rows updated / inserted to the command-line client Feb 12, 2016
@petermattis petermattis added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Feb 14, 2016
@petermattis petermattis modified the milestone: Beta Feb 14, 2016
@knz
Copy link
Contributor Author

knz commented Feb 18, 2016

So I was looking into this and again the DB interface is not helping.
The problem is that even with multiple result sets the interface is still either rows[][] or nothing per query. For reporting the row count for update and delete, we need a different kind of interface, where the number of rows is a separate return value from the rows themselves; i.e.

type ResultSet struct {
   OK bool
   size  int
   rows *[][]string
}
query(...)  (results []ResultSet, err error)

But this requires rather pervasive changes in the Go driver and db interface.

What is wise to do here?

  • Roll out our own extended interface in our fork of lib/pq?
  • Have update/delete return a single row containing the number of updates? (that would be very untypical SQL behavior)

@andreimatei
Copy link
Contributor

I thought returning the number of rows as the result is what @dt was working on? And I thought that's standard behavior? What does postgres return for INSERT and UPDATE?

@bdarnell
Copy link
Contributor

The trouble is that the go sql interface has a strict separation between Query (which returns rows) and Exec (which returns a count of rows affected). We need to either use a lower-level interface (i.e. use lib/pq directly without going through database/sql) that does not force us into this distinction or parse the statements on the client side so that we can send statements with results via Query and statements without results via Exec.

@petermattis
Copy link
Collaborator

Tomorrow (or early next week) I'm going to take a look at what going directly through lib/pq (or cockroach/pq) can buy us.

@knz
Copy link
Contributor Author

knz commented Feb 25, 2016

( While looking at #4036 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

4 participants