go-jet / jet

Type safe SQL builder with code generation and automatic query result data mapping
Apache License 2.0
2.23k stars 110 forks source link

Incorrect Docs: Generator Customization references invalid interfaces #351

Open PaluMacil opened 1 month ago

PaluMacil commented 1 month ago

Describe the bug

This issue is for improving the docs (if someone starts me on the right path, I'll also gladly make a PR once I understand), not the technical issue. I'm trying to use the directions for generator customization for a Postgres array of text.

In How to change model field type? > Generator customization, the text says the "type has to implement sql.Serializer and sql.Valuer interface." This is the only occurrence of Valuer or Serializer in the repo and the standard lib sql package doesn't have those interfaces either. I suspect this is incorrect. Do we need driver.Valuer? If so, what is the other interface needed?

Environment (please complete the following information): N/A (docs bug)

Code snippet The docs show two methods: Scan(value interface{}) error and Value() (driver.Value, error) but it's unclear what these do. The decimal.Decimal type from shopspring is referenced as working, but it's unclear why. Shopspring has enough methods that they might satisfy a number of interfaces, but the ones listed in the docs don't seem to be both present. driver.Valuer is satisfied, but I don't see anything like (d Decimal) Scan.

Things I tried (note, this issue is about documentation--I can make a separate one for code help or bugs if there turns out to be issues beyond the docs). This was my attempt to use a pgtype from pgx:

type TextArray struct {
    pgtype.Array[string]  // implements ArrayGetter and ArraySetter but nothing with Value or Scan, so probably not workable directly
}

...

        template.Default(pgdialect.Dialect).
            UseSchema(func(schema metadata.Schema) template.Schema {
                return template.DefaultSchema(schema).
                    UseModel(template.DefaultModel().
                        UseTable(func(table metadata.Table) template.TableModel {
                            return template.DefaultTableModel(table).
                                UseField(func(column metadata.Column) template.TableModelField {
                                    defaultTableModelField := template.DefaultTableModelField(column)

                                    if column.DataType.Name == "text[]" {
                                        defaultTableModelField.Type = template.NewType(types.TextArray{})
                                    }
                                    return defaultTableModelField
                                })
                        }),
                    )
            }),

Result:

I also noticed that when I stopped the debugger in column.DataType.Name, the Kind was base, not array. Not sure if I need to change that. I tried this type too, trying to emulate the examples, but I didn't get any different behavior from the unsupported column, using string I already saw:

type TextArray []string

func (ta *TextArray) Scan(value interface{}) error {
    switch v := value.(type) {
    case []byte:
        *ta = strings.Split(string(v[1:len(v)-1]), ",") // Removes the curly braces and splits the string
        return nil
    case string:
        *ta = strings.Split(v[1:len(v)-1], ",") // Same here
        return nil
    }
    return sql.ErrNoRows
}

func (ta TextArray) Value() (driver.Value, error) {
    return "{" + strings.Join(ta, ",") + "}", nil
}

Expected behavior A clear and concise description of what I need to do to add support for unsupported types such as text[]. Ideally, an example using types from the pgtypes in pgx would be amazing since it would open up 100+ types for use in Jet. If someone has an example of shimming in a couple types of pgtypes from pgx, I'll very gladly work on a docs PR!

houten11 commented 1 month ago

The documentation is correct "type has to implement sql.Scanner and sql.Valuer interface." sql.Scanner and sql.Valuer are go standard library interfaces and allow you to define how your custom types should be converted to and from SQL types. As you pointed out pgtype.Array[string] does not implement Scanner or Valuer, you are probably importing wrong pgx types which are not compatible with standard go sql library drivers. You'll need to use types from this repo - https://github.com/jackc/pgtype.

PaluMacil commented 1 month ago

@houten11 The documentation doesn't say anything about sql.Scanner. It says "type has to implement sql.Serializer and sql.Valuer interface." sql.Serializer isn't an interface. Should the docs be updated to say "sql.Scanner and sql.Valuer"? I got mixed up thinking Valuer was only in driver, not sql packages, so you're right about that one, I think.

Also, https://github.com/jackc/pgtype is deprecated and does not support pgx5:

This version is used with pgx v4. In pgx v5 it is part of the https://github.com/jackc/pgx repository. Only bug fixes will be made to this version. New features will only be considered for the current release.

The repo also says:

Supports database/sql.Scanner and database/sql/driver.Valuer interfaces for custom types which mixed me up on thinking the correct requirement is driver.Valuer. I think you're right though

go-jet commented 1 month ago

@houten11 The documentation doesn't say anything about sql.Scanner. It says "type has to implement sql.Serializer and sql.Valuer interface." sql.Serializer isn't an interface. Should the docs be updated to say "sql.Scanner and sql.Valuer"? I got mixed up thinking Valuer was only in driver, not sql packages, so you're right about that one, I think.

Yeah, you're right. It did say Serializer. I fixed it now.

PaluMacil commented 1 month ago

thanks @go-jet I think I'm set. Before I close this, do you think there might be some value in my making a PR for some more examples/docs or do you think it's good with your fix?

go-jet commented 1 month ago

@PaluMacil Yeah, off course.