postgres binding: added fix for uuid and numeric datatype columns and added two new methods fetchOne and fetchAll #3282
Conversation
… added two new methods fetchOne and fetchAll
|
|
||
| // Register uuid and decimal types with pgx | ||
| poolConfig.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error { | ||
| pgxuuid.Register(conn.TypeMap()) |
There was a problem hiding this comment.
I believe Postgres has built-in support for UUID via the google/uuid package that we use throughout Dapr. I'd rather not add another dependency here if possible
There was a problem hiding this comment.
Currently there is an issue with pgx for uuid and numeric columns which returns uuid or numeric columns as a array of strings #1016
You can find the details about the fix here https://github.com/jackc/pgx/wiki/UUID-Support
There was a problem hiding this comment.
I would rather using something that is based on https://pkg.go.dev/github.com/google/uuid please - that's the package we use throughout Dapr
| fetchOneOperation bindings.OperationKind = "fetchOne" | ||
| fetchAllOperation bindings.OperationKind = "fetchAll" |
There was a problem hiding this comment.
What's the difference between fetchOne, fetchAll, and query?
There was a problem hiding this comment.
query: returns array of strings (with no column names)
fetchOne: returns map of key value pairs (with column names as key)
fetchAll: returns array of map of key value pairs (with column names as key)
| for rows.Next() { | ||
| val, rowErr := rows.Values() | ||
| if rowErr != nil { | ||
| return nil, fmt.Errorf("error reading result '%v': %w", rows.Err(), rowErr) |
There was a problem hiding this comment.
rows.Err() and rowErr should be identical per docs
| return nil, fmt.Errorf("error reading result '%v': %w", rows.Err(), rowErr) | |
| return nil, fmt.Errorf("error reading row: %w", rowErr) |
| for rows.Next() { | ||
| val, rowErr := rows.Values() | ||
| if rowErr != nil { | ||
| return nil, fmt.Errorf("error reading result '%v': %w", rows.Err(), rowErr) |
There was a problem hiding this comment.
| return nil, fmt.Errorf("error reading result '%v': %w", rows.Err(), rowErr) | |
| return nil, fmt.Errorf("error reading row: %w", rowErr) |
| } | ||
|
|
||
| rs := make(map[string]any, 0) | ||
| for rows.Next() { |
There was a problem hiding this comment.
Since "fetchOne" is meant to fetch a single row, this method should return an error if the query returns more than one row.
Within the for loop, add a check to see if this is the 2nd iteration, and stop with an error if it is.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
ping @sridhar-shivraj |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
ping @sridhar-shivraj |
|
@sridhar-shivraj are you planning to work on this PR further? |
postgres binding: added fix for uuid and numeric datatype columns and added two new methods fetchOne and fetchAll
Description
Fixed the issue for uuid and numeric datatype columns, earlier uuid was coming as list of strings
Added two new methods
Issue reference
[https://github.com//issues/1016]
Please reference the issue this PR will close: #[1016]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: