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

Default Go types for nullable JSONB/UUID cannot handle null #745

Closed
soroushj opened this issue Oct 26, 2020 · 6 comments · Fixed by #1137
Closed

Default Go types for nullable JSONB/UUID cannot handle null #745

soroushj opened this issue Oct 26, 2020 · 6 comments · Fixed by #1137

Comments

@soroushj
Copy link
Contributor

soroushj commented Oct 26, 2020

The default Go types for nullable JSONB (json.RawMessage) and nullable UUID (uuid.UUID) cannot handle null. Using null with the former causes an error, while the latter cannot differentiate between zero value and null.

The example below demonstrates these issues. (Tested with sqlc v1.5.0, PostgreSQL v12, github.com/lib/pq v1.8.0, and github.com/google/uuid v1.1.2.)

Schema and queries:

create table nj (nj jsonb);
create table nu (nu uuid);

-- name: InsertNj :exec
insert into nj (nj) values (@nj);

-- name: GetNullNj :one
select nj from nj where nj is null limit 1;

-- name: InsertNu :exec
insert into nu (nu) values (@nu);

-- name: GetNullNu :one
select nu from nu where nu is null limit 1;

Demo code, assuming that:

  • queries is an instance of the generated Queries type; and
  • Both nu.nu and nj.nj columns contain at least one null row (for cases B and D).
// Case A: Cannot insert a null JSONB
err := queries.InsertNj(ctx, nil)
// err = "pq: invalid input syntax for type json"

// Case B: Cannot get a null JSONB
nj, err := queries.GetNullNj(ctx)
// err = "sql: Scan error on column index 0, name "nj": unsupported Scan, storing driver.Value type <nil> into type *json.RawMessage"

// Case C: Cannot insert a null UUID
// Using zero value of uuid.UUID, since it's an array and cannot be nil
err = queries.InsertNu(ctx, uuid.UUID{})
// Inserts "00000000-0000-0000-0000-000000000000"

// Case D: Cannot get a null UUID
nu, err := queries.GetNullNu(ctx)
// nu = "00000000-0000-0000-0000-000000000000"

How I worked around this issue

I implemented two new types, NullRawMessage and NullUUID, similar to Go's standard nullable SQL types, e.g., sql.NullString. It's available as a Go module: github.com/soroushj/sqlt.

sqlc can be configured to use these types:

{
  "version": "1",
  "packages": [],
  "overrides": [
    {
      "go_type": "github.com/soroushj/sqlt.NullRawMessage",
      "db_type": "jsonb",
      "nullable": true
    },
    {
      "go_type": "github.com/soroushj/sqlt.NullUUID",
      "db_type": "uuid",
      "nullable": true
    }
  ]
}

This solves the issue:

// Case A: Insert a null JSONB
err := queries.InsertNj(ctx, sqlt.NullRawMessage{})

// Case B: Get a null JSONB
nj, err := queries.GetNullNj(ctx)
// nj.Valid = false

// Case C: Insert a null UUID
err = queries.InsertNu(ctx, sqlt.NullUUID{})

// Case D: Get a null UUID
nu, err := queries.GetNullNu(ctx)
// nu.Valid = false

How sqlc can fix this issue

There are a few ways to address this issue.

A new module. As mentioned before, the module I open-sourced fixes the issue. But for security reasons, probably it's not a good idea for sqlc to generate code that imports a module maintained by a random contributor. You can fork/copy this module and change the default Go types.

Generate required types. Another option for sqlc is to generate an additional Go file that contains the implementation of the required types. No new import is required in this case.

I prefer the first option: The module can be maintained independently. For clients, a bug fix is a version bump, not a re-generate.

Also, there might be other ways to fix this issue, but I like the consistency of NullRawMessage and NullUUID with other nullable types.

@clstb
Copy link

clstb commented Jan 15, 2021

I experienced the same issue. Working with cockroachDB my primary keys are v4 UUID's. Having these types not nullable prevents me from using the UUID generator that is provided by cockroachDB.

Thanks for providing a work around @soroushj , I will use that for now.

EDIT: I experienced that sqlc will not change the type accordingly when you set nullable: true using yaml.

@soroushj
Copy link
Contributor Author

EDIT: I experienced that sqlc will not change the type accordingly when you set nullable: true using yaml.

@clstb I was not able to reproduce this issue using sqlc v1.6.0. There might be a mistake (e.g. bad indentation) in your YAML file.

This is the YAML equivalent of the sample JSON config above:

version: '1'
packages: []
overrides:
- go_type: github.com/soroushj/sqlt.NullRawMessage
  db_type: jsonb
  nullable: true
- go_type: github.com/soroushj/sqlt.NullUUID
  db_type: uuid
  nullable: true

@clstb
Copy link

clstb commented Jan 15, 2021

I am also using v1.6.0, this is my yaml file:

version: '1'
packages: []
overrides:
- go_type: github.com/soroushj/sqlt.NullUUID
  db_type: uuid
  nullable: true

The corresponding schema is:

CREATE TABLE IF NOT EXISTS accounts (
  id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
  name string NOT NULL
);

I am running Big Sur v11.1.

The concrete behaviour is that sqlc used the google uuid package in generated code. When omitting nullable it uses your package. I also tested with https://github.com/gofrs/uuid which does not show this behaviour.

Also I managed to fix my problem by omitting the primary key in creation queries, which is a cleaner solution for me at the moment.

@soroushj
Copy link
Contributor Author

@clstb In your case, that is the expected behavior. In PostgreSQL, PRIMARY KEY is equivalent to UNIQUE NOT NULL. As a result, when you set nullable: true for the uuid type override, it does not apply to the accounts.id column in your example. The default value for nullable is false--that's why the type override is used when you omit nullable.

@clstb
Copy link

clstb commented Jan 15, 2021

That makes sense, thank you for clarification 👍

@kyleconroy
Copy link
Collaborator

The UUID portion of this has been addressed in #1137. The JSON portion is a duplicate of #739. Don't worry though! The JSON side of things will also be fixed in v1.10.0

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 a pull request may close this issue.

3 participants