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

compiler: Support calling functions with defaults #635

Merged
merged 1 commit into from
Jul 29, 2020
Merged

Conversation

kyleconroy
Copy link
Collaborator

@kyleconroy kyleconroy commented Jul 29, 2020

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

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) {
Copy link
Collaborator Author

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) {
Copy link
Collaborator Author

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)
Copy link
Collaborator Author

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) {
Copy link
Collaborator Author

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) {
Copy link
Collaborator Author

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())
Copy link
Collaborator Author

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.

@kyleconroy kyleconroy merged commit d2541ce into master Jul 29, 2020
@kyleconroy kyleconroy deleted the calling-funcs branch July 29, 2020 17:28
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

Successfully merging this pull request may close these issues.

1 participant