use with github.com/volatiletech/null/v9 panics

[msonawane]

Describe the bug
A clear and concise description of what the bug is.

if model has null.String fields INSERT panics with
panic: jet: null.String type can not be used as SQL query parameter

at internal/jet/sql_builder.go:238 due to

default:
		if strBindValue, ok := bindVal.(toStringInterface); ok {
			return stringQuote(strBindValue.String())
		}
		panic(fmt.Sprintf("jet: %s type can not be used as SQL query parameter", reflect.TypeOf(value).String()))
	}

Environment (please complete the following information):

• OS :Linux
• Database: postgres
• Database driver: pgx
• Jet version: 2.7.1

Code snippet

import (
	"time"

	"github.com/volatiletech/null/v9"
)

type OrgUser struct {
	ID           int64 `sql:"primary_key"`
	Name         string
	IsEmployee   *bool
	Salutation   null.String
	FirstName    null.String
	LastName     null.String
	Telephone    null.String
	Mobile       null.String
}
stmt := table.OrgUser.INSERT(table.OrgUser.AllColumns).
		MODEL(user)
result, err := stmt.ExecContext(ctx, db)

Expected behavior
null.String implements scanner and valuer interfaces. insert should work

[go-jet]

Hi @msonawane. Panicking function argToString is called only if .DebugSql() is called. Perhaps you are calling .DebugSql() from logger function. If you remove the call or replace it with .Sql() insert should work.

But still, we should be able to create debug sql from valuer interface. The fix would probably be, something like this:

	if valuer, ok := bindVal.(driver.Valuer); ok {
		val, err := valuer.Value()

		if err != nil {
			return err.Error()
		}

		return argToString(val)
	}

	panic(fmt.Sprintf("jet: %s type can not be used as SQL query parameter", reflect.TypeOf(value).String()))

[msonawane]

@go-jet thank you

Looks like this will impact all other types which are not in type switch (null.Int64) etc. Any suggestions ?
I would like to understand your views on string pointers used in go-jet as they are now. is it commonly done ?

another issue I have is to identify errors coming from database to better represent state on front end and logs etc. right now i am having few functions to classify errors like

func IsUniqueViolation(err error) (bool, string) {
	var pgErr *pgconn.PgError
	if errors.As(err, &pgErr) {
		
		if pgErr.Code == "23505" {
			fmt.Printf("#%v\n", pgErr.ConstraintName)
			return true, pgErr.ConstraintName
		}
	}
	return false, ""
}

ideally this should be part of go-jet but thats specific to drivers used. What should be a good way to handle this

[go-jet]

Looks like this will impact all other types which are not in type switch (null.Int64) etc. Any suggestions ?

The solution should work for all Valuer types, not just null.String, because value is first converted to Valuer interface and then whatever value is inside, is passed to argToString again.

would like to understand your views on string pointers used in go-jet as they are now. is it commonly done ?

I personally don't see any benefit of using null types over raw pointers. I always use raw pointers, but I did saw a couple of projects using null types.

another issue I have is to identify errors ....

Yeah, that's probably preferred way to handle it. My view: #109 (comment)

[msonawane]

@go-jet thanks for your inputs. will it be possible to generate all constraints on table so users will have a type safe way to check for them ?

[go-jet]

Other constraints, like UNIQUE, PRIMARY KEY, FOREIGN KEY and CREATE INDEX can only be ensured on the database side.
CHECK and DEFAULT can be ensured on the server side, but there is little benefit of doing that. Because that wouldn't be a compile time check/set. It would be duplicate runtime check/set on server and database side.

Which constraint did you have in mind?

[go-jet]

Fixed with Release 2.8.0.