Improvement on PGX nullable type usage

[Oliver-Fish]

Inside the readme, the following is found

Choosing an appropriate type is more difficult than might seem at first glance due to null. When Postgres reports that a column has a type text, that column can have both text and null values. So, the Postgres text represented in Go can be either a string or nil. pgtype provides nullable types for all built-in Postgres types. pggen tries to infer if a column is nullable or non-nullable. If a column is nullable, pggen uses a pgtype Go type like pgtype.Text. If a column is non-nullable, pggen uses a more ergonomic type like string. pggen's nullability inference in internal/pginfer/nullability.go is rudimentary; a proper approach requires a full explain plan with some control flow analysis.

However, if you use PGX directly and not the standard lib SQL version, the null types from pgtype are rarely required. Instead, you can simply use the native type with a pointer to infer what is optional and what is not. Is this package designed to support PGXs usage of the raw postgres protocol? If so it would be great to re-visit when pgtypes actually need to be used.

[jschaf]

Great questions! I'll answer inline:

• "if you use pgx directly ... Is this package designed to support PGXs" - Yes, pggen uses pgx exclusively and avoids the Go std library support for SQL.
• "you can simply use the native type with a pointer" - Yep, pggen currently supports this but only across an entire query file with --go-type 'text=*string'. Or, if you want to skip pointers, you can also use --go-type 'text=string'

The original rationale for using the pgtype types instead of pointers like *string was for consistency: if you have null types the only out-of-the-box method was to use a pgtype. pgx only supports a limited number of native pointer types, mainly *string and the int types (*int16, etc...).

I'd interpret this as a feature request to use *string instead of pgtype.Text for nullable string outputs. Similarly, pggen should use int pointers instead of pgtype.Int8 and friends. For everything else, keep using pgtype. Does that match your thinking?

[Oliver-Fish]

Yep, that is exactly matching my thought process here, part of the reason I migrated a previous code-based to native PGX was to avoid using these nullable types and to use native ones instead because the development ergonomics are much improved by this.

[jschaf]

I can't figure out how to use a string pointer to scan a null value, pgx doesn't support scanning null in the default string scanner.

func TestPGXScanNull(t *testing.T) {
    conn, cleanup := pgtest.NewPostgresSchema(t, []string{})
    defer cleanup()
    ctx := context.Background()

    row := conn.QueryRow(ctx, `SELECT null::text`)
    val := ""
    if err := row.Scan(&val); err != nil {
        t.Error(err)
    }
}

This errors with: can't scan into dest[0]: cannot scan null into *string.

Seems like there should be a way to make this work. If there's not then I don't see an easy way to add support for *string instead of pgtype.Text. We could do the transformation in the generated code where we scan with pgtype.Text and then assign it a *string. I think that approach will be difficult once you factor in arrays and composite types.

[Oliver-Fish]

Looks like your making a slight error here, you must use a type of *string you can't use a pointer to a string. In the above case, I am not sure how it would actually be able to handle writing null to that string.

Here is some POC code I threw together to show you how it works

    var val *string
    row := conn.QueryRow(ctx, `SELECT null::text`)
    if err := row.Scan(&val); err != nil {
        panic(err)
    }
    fmt.Println(val)

    row = conn.QueryRow(ctx, `SELECT 'aa'::text`)
    if err := row.Scan(&val); err != nil {
        panic(err)
    }
    fmt.Println(*val)

Output

<nil>
aa

[jschaf]

Looks like your making a slight error here, you must use a type of *string you can't use a pointer to a string. In the above case, I am not sure how it would actually be able to handle writing null to that string.

Thanks, I thought I tried the double pointer. Not sure what I was doing wrong.

Anyway, I've implemented this in 00aa03d. Give it a go and report back if anything is broken.