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

Incorrect batchone QueryRow method when using emit_result_struct_pointers #2203

Closed
ericbock opened this issue Apr 12, 2023 · 4 comments
Closed
Labels
bug Something isn't working triage New issues that hasn't been reviewed

Comments

@ericbock
Copy link

Version

1.17.2

What happened?

When using :batchone with pgx/v4 and the "emit_result_struct_pointers": true, the QueryRow method does not use a pointer for the model when closed and calling the callback:

func (b *GetAuthorBatchResults) QueryRow(f func(int, *Author, error)) {
	defer b.br.Close()
	for t := 0; t < b.tot; t++ {
		var i Author
		if b.closed {
			if f != nil {
				f(t, i, errors.New("batch already closed"))
			}
			continue
		}
		row := b.br.QueryRow()
		err := row.Scan(&i.ID, &i.Name, &i.Bio)
		if f != nil {
			f(t, &i, err)
		}
	}
}

Here, the callback expects an *Author, but it's getting f(t, i, ...) where i is an Author, not an *Author.

I think this should be

		if b.closed {
			if f != nil {
				f(t, &i, errors.New("batch already closed"))
			}
			continue
		}

Relevant log output

graph/store/batch.go:148:10: cannot use i (variable of type BatchGetSkusRow) as *BatchGetSkusRow value in argument to f

Database schema

CREATE TABLE authors (
  id   BIGSERIAL PRIMARY KEY,
  name text      NOT NULL,
  bio  text
);

SQL queries

-- name: GetAuthor :batchone
SELECT * FROM authors
WHERE id = $1 LIMIT 1;

Configuration

{
  "version": "2",
  "sql": [
    {
      "schema": "query.sql",
      "queries": "query.sql",
      "engine": "postgresql",
      "gen": {
        "go": {
          "out": "db",
          "sql_package": "pgx/v4",
	  "emit_result_struct_pointers": true
        }
      }
    }
  ]
}

Playground URL

https://play.sqlc.dev/p/303a2261c50646777b637a96f62d00ede88494b8dda5cb88f3311eee3d9b8e99

What operating system are you using?

macOS

What database engines are you using?

PostgreSQL

What type of code are you generating?

Go

@ericbock ericbock added bug Something isn't working triage New issues that hasn't been reviewed labels Apr 12, 2023
@andrewmbenton
Copy link
Collaborator

Which version of sqlc are you using? When I run codegen on this example I get the following, so this may have been a bug that was previously fixed.

		if b.closed {
			if f != nil {
				f(t, nil, ErrBatchAlreadyClosed)
			}
			continue
		}

@ericbock
Copy link
Author

This is with v1.17.2

It looks like the batchCode template was updated recently (in e505aec), but besides defining an error type I don't see a change in the diff that would fix the problem.

@andrewmbenton
Copy link
Collaborator

andrewmbenton commented Apr 26, 2023

Looking back before that most recent update to the template, there was a patch merged that addressed this issue. There is discussion on this prior issue thread: #1959

Based on the activity in that thread, I think the patch did not make it into the 1.17.2 release (it was merged one day after). But it should be in the next release.

@kyleconroy
Copy link
Collaborator

Fixed in v1.18.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage New issues that hasn't been reviewed
Projects
None yet
Development

No branches or pull requests

3 participants