Wrong null Time type when emit_pointers_for_null_types is set to true for pgx/v5

[dandee]

Version

1.21.0

What happened?

When query_parameter_limit > 0 and emit_pointers_for_null_types is set to true sqlc generates parameter type of DB engine specific time type instead of *time.Time:

pgx/v5: func (q *Queries) UpdateLoginTime(ctx context.Context, uid uuid.UUID, lastLogin pgtype.Timestamptz) (User, error) {

I expect type for lastLogin to be *time.Time.

Database schema

CREATE TABLE IF NOT EXISTS users
(
    id SERIAL PRIMARY KEY,
    first_login timestamp with time zone,
    last_login timestamp with time zone
);

SQL queries

-- name: UpdateLoginTime :one
UPDATE users
  SET first_login = CASE WHEN first_login IS NULL THEN $2 END,
  last_login = $2
WHERE uid = $1
RETURNING *;

Configuration

version: "2"
sql:
- engine: "postgresql"
  queries: "storage"
  schema: "storage/migrations"
  gen:
    go:
      sql_package: "pgx/v5"
      package: "db"
      out: "db"
      emit_json_tags: true
      emit_pointers_for_null_types: true
      json_tags_case_style: camel
      query_parameter_limit: 5
      overrides:
      - db_type: "uuid"
        go_type: "github.com/gofrs/uuid.UUID"
      - go_struct_tag: "json:\"-\""
        column: "*.password"

Playground URL

No response

What operating system are you using?

macOS

What database engines are you using?

PostgreSQL

What type of code are you generating?

Go

[orisano]

Same as #2459

This issue will be resolved at #2597.

[dandee]

Oh, I'm sorry, I've tried to find existing issue for that but I couldn't. Is it also going to address problem with incorrect pgtype.Timestamptz instead of *time.Time for emit_pointers_for_null_types: true?

[dandee]

Same as #2459

This issue will be resolved at #2597.

I've edited the description to remove the issue fixed in the 1.21.0 release and kept the one that still exists.

[andrewmbenton]

I don't think query_parameter_limit is related, as I see the same behavior with it set to 0.

I also don't think the value of emit_pointers_for_null_types has any effect on how sqlc determines input parameter types, but I might be wrong about that. And arguably it should I suppose. That could be raised as a new enhancement issue.

I think you can get the behavior you want with an override though. See the playground link below. Note that for the example to work I had to define column types as timestamptz because sqlc apparently doesn't recognize timestamp with time zone as type timestamptz when considering overrides.

https://play.sqlc.dev/p/46304e09afdea98104a6b7c2e4ec0aae63cfb17104714e3fceda8a4fa1e0c457

[dandee]

Well, documentation on emit_pointers_for_null_types is pretty clear on that I think:

emit_pointers_for_null_types:

If true and sql_package is set to pgx/v4 or pgx/v5, generated types for nullable columns are emitted as pointers (ie. *string) instead of database/sql null types (ie. NullString). Defaults to false.

So according to the above not outputting *time.Time is a bug (or *uuid.UUID in this case: #2738)

Thanks for the tip for timestamptz. Should I create another bug for not recognizing timestamp with time zone as timestampz?

[andrewmbenton]

Well, documentation on emit_pointers_for_null_types is pretty clear on that I think:

emit_pointers_for_null_types:

If true and sql_package is set to pgx/v4 or pgx/v5, generated types for nullable columns are emitted as pointers (ie. *string) instead of database/sql null types (ie. NullString). Defaults to false.

So according to the above not outputting *time.Time is a bug (or *uuid.UUID in this case: #2738)

I think this is correct, having looked a little deeper. I'll update the title of this issue and reopen. There is an early return in

sqlc/internal/codegen/golang/postgresql_type.go

Lines 247 to 258 in ac32049

	case "pg_catalog.timestamptz", "timestamptz":
	if driver == SQLDriverPGXV5 {
	return "pgtype.Timestamptz"
	}
	if notNull {
	return "time.Time"
	}
	if emitPointersForNull {
	return "*time.Time"
	}
	return "sql.NullTime"

which is causing this, and there are a few other similar early returns that need cleaning up.

Thanks for the tip for timestamptz. Should I create another bug for not recognizing timestamp with time zone as timestampz?

Yes I think so, thanks.

[Flamefork]

Just for reference, here is all types that has SQLDriverPGXV5 logic preceding (thus overriding) emit_pointers_for_null_types one:

• date
• pg_catalog.time
• pg_catalog.timestamp
• pg_catalog.timestamptz
• timestamptz
• uuid
• interval
• pg_catalog.interval

[dandee]

Created as a separate issue here: #2914