Skip to content

A computed column's nullability is not inferred, so a correlated subquery that matches nothing fails at scan time instead of at declaration time #147

Description

@jryannel

Porting a module whose list did LEFT JOIN projects p ON te.project_id = p.id and projected p.name. Declared as a computed column:

schema.Computed("project_name", schema.TypeText,
    schema.FromSQL("(SELECT p.name FROM projects p WHERE p.id = time_entries.project_id)")).
    Searchable(),

Generated model:

ProjectName string `db:"project_name" ... sqlb:"type:text,...,readonly"`

First request against real data:

rest: unclassified error answering as 500 resource=time_entry
err="sqlb: scanning timeentriesdata.TimeEntry: can't scan into dest[17] (col: project_name): cannot scan NULL into *string"

.Nullable() fixes it and the port carries that now. The report is about where the failure lands.

Why non-null is the wrong default here

A correlated subquery returns NULL when it matches nothing. For this column that is not an edge case — it is every row with no project, which is most of them, plus every row pointing at a deleted project, which is possible precisely because a cross-module reference has no FK. The LEFT JOIN this replaced had exactly these semantics and the hand-written DTO spelled the field *string.

So the declaration defaulted to the one reading the expression cannot satisfy, and it did it silently. A stored column gets its nullability from NOT NULL in the DDL, which the round trip checks; a computed column has no DDL to read it from, so the default is doing all the work and it points the wrong way for the shape that motivated the feature.

Why the diagnosis is more expensive than it looks

  • It surfaces as a 500 at runtime, not as an error from sqlb generate or from the drift gate. Both were green: generation has no opinion, and migrate.Diff correctly ignores a column that is not in the database.
  • It needs data to reproduce. A test fixture where every row happens to have a project passes. Ours only failed because one test seeded an entry without one.
  • The message names the Go type, not the declaration. cannot scan NULL into *string sends you to the model, which is generated, before it sends you to the Computed call that produced it.

Suggested fix, in order

  1. Default a computed column to nullable, and let NotNull() (or similar) be the opt-in. Any expression can be NULL unless the author says otherwise; the current default assumes the opposite of what SQL does.
  2. Or infer it. A (SELECT …) subquery is nullable; a AND b over two non-null columns is not. Probably more machinery than it is worth, and wrong in the unsafe direction whenever the inference is incomplete.
  3. Or refuse the ambiguity: require every Computed to state nullability explicitly, so neither default can be silently wrong. Breaking, but the declaration is where this belongs and there are not many of them in any one schema.

1 is what I would want. It matches SQL's own default and it fails in the safe direction: a nullable column typed *string scans a non-null value fine, where the reverse does not.

Related

Same port as #142, #143, #144, #146. This one and #144 are the two that cost real debugging time; the rest announced themselves.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions