-
Notifications
You must be signed in to change notification settings - Fork 802
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
compiler: Support calling functions with defaults #635
Conversation
Fix a number of bugs when resolving function calls. Switch to using functions generated from a default PostgreSQL instance. Add a new test case ripped from the PostgreSQL docs.
@@ -11,9 +11,9 @@ const makeIntervalDays = `-- name: MakeIntervalDays :one | |||
SELECT make_interval(days => $1::int) | |||
` | |||
|
|||
func (q *Queries) MakeIntervalDays(ctx context.Context, dollar_1 int32) (interface{}, error) { | |||
func (q *Queries) MakeIntervalDays(ctx context.Context, dollar_1 int32) (int64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make_interval
now returns an actual type instead of interface{}
. This happened because we can now resolve function calls with defaults and named parameters.
@@ -33,9 +33,9 @@ const makeIntervalSecs = `-- name: MakeIntervalSecs :one | |||
SELECT make_interval(secs => $1) | |||
` | |||
|
|||
func (q *Queries) MakeIntervalSecs(ctx context.Context, secs interface{}) (interface{}, error) { | |||
func (q *Queries) MakeIntervalSecs(ctx context.Context, secs float64) (int64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler is now smart enough to pull out type information for named parameters in function calls.
} | ||
|
||
func (q *Queries) Plus(ctx context.Context, arg PlusParams) (int32, error) { | ||
row := q.db.QueryRowContext(ctx, plus, arg.B, arg.A) | ||
row := q.db.QueryRowContext(ctx, plus, arg.A, arg.B) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!!!
The compiler generated incorrect code here. The order of the arguments wasn't correct.
@@ -17,15 +17,15 @@ type GenerateSeriesParams struct { | |||
Column2 time.Time `json:"column_2"` | |||
} | |||
|
|||
func (q *Queries) GenerateSeries(ctx context.Context, arg GenerateSeriesParams) ([]interface{}, error) { | |||
func (q *Queries) GenerateSeries(ctx context.Context, arg GenerateSeriesParams) ([]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Goodbye interface{}
! Real return values for function calls
@@ -11,8 +11,8 @@ const advisoryLock = `-- name: AdvisoryLock :many | |||
SELECT pg_advisory_unlock($1) | |||
` | |||
|
|||
func (q *Queries) AdvisoryLock(ctx context.Context, key int64) ([]bool, error) { | |||
rows, err := q.db.QueryContext(ctx, advisoryLock, key) | |||
func (q *Queries) AdvisoryLock(ctx context.Context, pgAdvisoryUnlock int64) ([]bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the PostgreSQL documentation claims the first parameter to pg_advisory_unlock
is named key
, that name isn't actually included in the definition of the function stored in the database.
@@ -5,7 +5,7 @@ import "github.com/kyleconroy/sqlc/internal/sql/catalog" | |||
func NewCatalog() *catalog.Catalog { | |||
c := catalog.New("public") | |||
c.Schemas = append(c.Schemas, pgTemp()) | |||
c.Schemas = append(c.Schemas, pgCatalog()) | |||
c.Schemas = append(c.Schemas, genPGCatalog()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit scary! I'm curious if we'll have to walk this specific change back.
Fix a number of bugs when resolving function calls.
Switch to using functions generated from a default PostgreSQL instance.
Add a new test case ripped from the PostgreSQL docs.
I've noted the improvements in individual comments