stephenafamo/bob

Arg and ArgGroup enchantments to accept any slice

atzedus opened this issue · 5 comments

Very frequency case is to use construction like this:

ids := []string{"a", "b", "c"}

dst := make([]any, len(ids))
for i := range ids {
	dst[i] = ids[i]
}

psql.Quote("field").In(psql.Arg(dst...))

I think that it can be improved with small code changes like this in file:
https://github.com/stephenafamo/bob/blob/main/dialect/psql/starters.go#L61-L69

// SQL: $1, $2, $3
// Go: psql.Args("a", "b", "c")
func Arg[T any](args ...T) Expression {
	return bmod.Arg(toAny(args)...)
}

// SQL: ($1, $2, $3)
// Go: psql.ArgGroup("a", "b", "c")
func ArgGroup[T any](args ...T) Expression {
	return bmod.ArgGroup(toAny(args)...)
}

func toAny[T any](src []T) []any {
	dst := make([]any, len(src))
	for i := range src {
		dst[i] = src[i]
	}
	return dst
}

On the other hand, it may be a bad idea to perform this manipulation with the slice every time, even when it may not be required.

In this case, perhaps the best solution would be to simply add new methods (with 's') like:

// SQL: $1, $2, $3
// Go: psql.Args("a", "b", "c")
func Args[T any](args ...T) Expression {
	return Arg(toAny(args)...)
}

// SQL: ($1, $2, $3)
// Go: psql.ArgsGroup("a", "b", "c")
func ArgsGroup[T any](args ...T) Expression {
	return ArgGroup(toAny(args)...)
}

func toAny[T any](src []T) []any {
	dst := make([]any, len(src))
	for i := range src {
		dst[i] = src[i]
	}
	return dst
}

What do you think about it?

I would prefer not to duplicate these functions. Brings a lot of user confusion over which one to use.

Not sure if to modify the base args in expr/arg.go, but this will of course be slightly lower because it will need to create a new slice each time.

So, modifying expr/arg.go does not make sense, because psql.Arg(args ...any) for example will still not accept anything than "any" type.

Perhaps then the internal.ToAnySlice() function deserves to be part of the bob package? Don't you think so?

Another case i found where it may be usefull is for example OnConflict(columns ...any) function, like:

conflictCols := []dialect.Expression{
	psql.Quote("name"),
	psql.Quote("code"),
}
......
psql.Insert(
	im.OnConflict(bob.ToAnySlice(conflictCols)...),
)

Yes, it stupid example, because we can simply define it as conflictCols := []any{}, but I'm sure that there may be a situation where typing will be important.

So, modifying expr/arg.go does not make sense, because psql.Arg(args ...any) for example will still not accept anything than "any" type.

Well.... if we decided to go this way, we could bubble this up to psql.Arg (and the other dialects).

Perhaps then the internal.ToAnySlice() function deserves to be part of the bob package? Don't you think so?

While this would be convenient, I am very reluctant to export anything that is not core to Bob since being part of the public API of the package means maintaining it forever.
While it is a "small function" these things very quickly add up, and then we have to maintain all these helper functions that are not really core to the package.

I believe such helper functions should either be written by the user, or provided by a package dedicated to helper functions.
For example, samber/lo has a ToAnySlice function.

Another case i found where it may be usefull is for example OnConflict(columns ...any) function, like:

Side note: You can achieve the same with orm.NewColumns:

psql.Insert(
	im.OnConflict(orm.NewColumns("name", "code")), // "name" AS "name", "code" AS "code"
)

You can also add a parent to all columns or an alias prefix.

orm.NewColumns("name", "code").WithParent("a").WithPrefix("a__"),
// "a"."name" AS "a__name", "a"."code" AS "a__code"

Well.... if we decided to go this way, we could bubble this up to psql.Arg (and the other dialects).

I will be happy if you accept this changes. I can prepare PR soon :)
For the sqlboiler I had a small package with various querymods extensions, but for the bob it is completely useless, because... everything is in the core, except for this one.

While this would be convenient, I am very reluctant to export anything that is not core to Bob since being part of the public API of the package means maintaining it forever.
While it is a "small function" these things very quickly add up, and then we have to maintain all these helper functions that are not really core to the package.

I completely agree with this.

I will be happy if you accept this changes. I can prepare PR soon :)

After giving this more thought, I have to turn it down.

Making this generic means duplicating every argument in every query and I don't feel comfortable with that.

In the future, if Go adds an easy way to switch on generic types then we could reconsider this because in that case we could at least skip the arg duplication if the slice is already []any