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

feat: ResultBytes #86

Merged
merged 5 commits into from
Aug 16, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions sqlitex/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import (
"zombiezen.com/go/sqlite"
)

var errNoResults = errors.New("sqlite: statement has no results")
var errMultipleResults = errors.New("sqlite: statement has multiple result rows")
var (
ErrNoResults = errors.New("sqlite: statement has no results")
ErrMultipleResults = errors.New("sqlite: statement has multiple result rows")
)
Copy link
Owner

Choose a reason for hiding this comment

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

Let's revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I get some clarification on this? Generally, I believe that sentinel errors should be exported if they have meaningful names and convey meaningful information. In this case, they would allow the caller the option to distinguish between "the call I explicitly expected to have one result has none" and " the call I explicitly expected to have one result has more than one".

If the argument is that you deliberately want this information to be opaque, I can get behind that I suppose. The semantics of the methods that might return these errors are clear enough ("one result, or error"). That would make these errors not so much sentinel errors but helpful templates. I just want to make sure I'm clear on the reasoning behind the decision to hide this distinction.

Copy link
Owner

Choose a reason for hiding this comment

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

Yup, sorry for being terse. I'm asking to revert from the standpoint that ResultBytes is useful on its own (and the title/focus of the PR) and I would like to merge it in. I am less sure about exporting the errors at this time, so would like to figure that outside the scope of this PR.


func resultSetup(stmt *sqlite.Stmt) error {
hasRow, err := stmt.Step()
Expand All @@ -20,7 +22,7 @@ func resultSetup(stmt *sqlite.Stmt) error {
}
if !hasRow {
stmt.Reset()
return errNoResults
return ErrNoResults
}
return nil
}
Expand All @@ -33,7 +35,7 @@ func resultTeardown(stmt *sqlite.Stmt) error {
}
if hasRow {
stmt.Reset()
return errMultipleResults
return ErrMultipleResults
}
return stmt.Reset()
}
Expand Down Expand Up @@ -100,3 +102,16 @@ func ResultFloat(stmt *sqlite.Stmt) (float64, error) {
}
return res, nil
}

// ResultBytes reads the first column of the first and only row produced by running stmt into buf, returning the number of bytes read.
// It returns an error if there is not exactly one result row.
func ResultBytes(stmt *sqlite.Stmt, buf []byte) (int, error) {
zombiezen marked this conversation as resolved.
Show resolved Hide resolved
if err := resultSetup(stmt); err != nil {
return 0, err
}
read := stmt.ColumnBytes(0, buf)
if err := resultTeardown(stmt); err != nil {
return 0, err
}
return read, nil
}