Nullability seemingly not respected when only one string column is returned

[Gbps]

Hey there, great project, I'm getting a lot of use out of it.

Here is the problem query:

-- name: xxxGetLogs :one
select
    string_agg(t.format, e'\n') as logs
from
    (
        select
            format(
                '[%s] %s',
                to_char(
                    time at time zone 'utc' at time zone 'america/los_angeles',
                    'MM-DD-YY HH12:MI:SS AM'
                ),
                line
            ) as format
        from
            table2
        where
            parse_id = (
                select
                    id
                from
                    table1
                where
                    val = pggen.arg ('val')
            )
    ) as t;

Which generates this code:

func (q *DBQuerier) xxxGetLogs(ctx context.Context, val string) (*string, error) {
    ctx = context.WithValue(ctx, "pggen_query_name", "xxxGetLogs")
    row := q.conn.QueryRow(ctx, xxxGetLogsSQL, val)
    var item string
    if err := row.Scan(&item); err != nil {
        return &item, fmt.Errorf("query xxxGetLogs: %w", err)
    }
    return &item, nil
}

Everything seems to be working great except that row.Scan is reading into var item string instead of var item *string since it can be nullable. The functionality works as expected if there are two columns (it reads to a struct where the column is *string) but when it's a single column return, it seems to force rows.Scan to read into string instead of *string.

I understand nullability inference is hard but that's not the problem here, since I believe it thinks this column should be nullable which is correct, but it just isn't generating the right code to read it into a nullable golang type.

Any suggestions?

[jschaf]

Thanks for reporting. The nullability is pretty limited but it should never be wrong like this. I suspect you're hitting triggering this case: https://github.com/jschaf/pggen/blob/main/internal/pginfer/nullability.go#L30.

Can you post the explain plan output?

[Gbps]

Sorry for the late response, here it is:

Aggregate  (cost=3.46..3.47 rows=1 width=32)
  InitPlan 1 (returns $0)
    ->  Seq Scan on table1 (cost=0.00..1.01 rows=1 width=4)
          Filter: ((val)::text = 'abc'::text)
  ->  Seq Scan on table2 (cost=0.00..2.23 rows=18 width=78)
        Filter: (parse_id = $0)

[jschaf]

Oh, the nullability doesn't account for the fact that aggregating a non-null column with no joins can still be null if no rows match.

[Gbps]

Gotcha, understood. However, I'm still a bit confused since I thought that the default for when nullability was unknown that it would fall back to a nullable golang type. Is that not the case?

Is there a way I can explicitly tell the codegen that a particular arg is nullable or not? Maybe a second argument to pggen.arg?

[jschaf]

However, I'm still a bit confused since I thought that the default for when nullability was unknown that it would fall back to a nullable golang type. Is that not the case?

It turns out the nullability is working as intended. To verify, I added a test case in the linked commit, f664a91. I suspect the problem occurs when templating the Go code. There's some complexity in unwrapping types to get the variable declaration. We're probably aggressively pruning the type too aggressively. Consequently, we end up dropping the pointer type wrapper on the underlying string type. I'll investigate.

Is there a way I can explicitly tell the codegen that a particular arg is nullable or not? Maybe a second argument to pggen.arg?

No. I've tried very hard to avoid adding knobs since it complicates testing. Long term, I'd like to add a configuration system with the following traits.

• A config for all queries in a single pggen invocation: (PostgresType, GoType, optional conversion method)
• A config for a single query (PostgresType, GoType, optional conversion method)
• A config for a single column in a query: (GoType, optional conversion method)

[jschaf]

I cut a new release, https://github.com/jschaf/pggen/releases/tag/2022-12-15, with the fix. Let me know if that doesn't do the trick.

[Gbps]

Thank you for the fix! It looks like all is well now with queries using :one, however the same problem seems to be present when converting the same kind of query to :many. Just converting the same query to :many should suffice for a repro.