Better support for Prepare

[RangelReale]

In the documentation about Prepare, it says to create a statement using Prepare and then executing the statement sending the arguments.

The problem with it is that we don't have a good way to know in what order the arguments should be sent other than trial-and-error or guessing, because the query is generated.

I created a proof-of-concept (brainstorm, not production code) of creating named args that could be replaced with a map at execution time, outputting an array in the correct order.

What do you think?

[stephenafamo]

I see a need for something like this. It actually had not occurred to me that the order of args may be unexpected for prepared statements.

At first glance, I'm not totally in love with your proof-of-concept, the way to use named args doesn't seem completely intuitive to me. However, it is possible that this is the best way to make it work.

I'll try to think about it, but I have a lot going on over the coming months, so this would likely not be something I tackle soon.

[RangelReale]

On the matter of using a struct for parameters, we would need a standard way of converting the struct fields to parameter names (snake case only?).

sqlx uses the same mapper it uses for reading data into structs to map input params struct fields, but your scan package seems to need to be tied to an specific instance, and I don't think I should import this package inside bob directly (nevermind, it is already imported), do you have some other way of reading struct values in the repo?

[stephenafamo]

Also sql.NamedArg doesn't specify the format the named parameter should be (:param, @param, etc), this shows even more that it is meant to be server/driver specific.

Since Bob generates the query itself using dialect.WriteArg, then it is up to Bob to determine the format in the query. The only place to take note would be in Raw(), and even then Bob rewrites the query anyway.

So, for these servers, the sql.NamedArg value must reach them, so we can't intercept it for other uses, so I think it would not be a good thing to use it.

With Bob's driver system, we can send the NamedArg to the drivers that support it and rewrite it to numbered args for the dialects that do not.

With NamedArg, Swish can separate the name and the arg and rewrite them appropriately, similar to how sqlx rewrites queries with :named. Although perhaps to avoid confusion, Bob should use its own NamedArg instead of the one provided by database/sql 🤔

On the matter of using a struct for parameters, we would need a standard way of converting the struct fields to parameter names (snake case only?).

A lot of these utilities are in the scan package. Some are also in the internal package and is used for pulling values from models.

[RangelReale]

then it is up to Bob to determine the format in the query

We won't be generating a named parameter in the SQL itself, the query will still contain DB-specific placeholders ($1, $2), otherwise we would need to generate the same query 2 times, one with named parameters, and one without, and the first one would have no use, as the server would not accept it.

A lot of these utilities are in the scan package.

The scan package seems very tied to Rows, I'll see if I can get the information from it.

[RangelReale]

Looks like I could use the scan package if some of the mapping methods were public, how do you feel about doing this?

	dt := Data{
		FirstName: "aa",
		LastName:  "bb",
	}

	sm, _ := scan.NewStructMapperSource()
	m, err := sm.GetMapping(reflect.TypeOf(dt))
	if err != nil {
		panic(err)
	}

	for _, xm := range m {
		fmt.Println(xm.Name)
	}

	v := reflect.ValueOf(dt)
	for _, info := range m {
		fmt.Println(info.Name)
		fv := v.FieldByIndex(info.Position)
		fmt.Println(fv.Interface())
	}

[stephenafamo]

We won't be generating a named parameter in the SQL itself, the query will still contain DB-specific placeholders ($1, $2), otherwise we would need to generate the same query 2 times, one with named parameters, and one without, and the first one would have no use, as the server would not accept it.

Exactly. Since the query is generated "on demand", we can decide how this happens for each dialect.

Looks like I could use the scan package if some of the mapping methods were public, how do you feel about doing this?

Should be fine, similar to Bob, a lot of the methods/types in the scan package a left private to prevent future breaking changes.

[RangelReale]

Here is my second proof-of-concept, this is the general ideia (there's a sample.go file at the bottom showing how calling it looks like):

• move the WriteQuery method of Query to a separate interface (QueryWriter), and embed it in Query
• create a PreparedQuery interface, that will be returned from BaseQuery.BuildPrepared(). This interface has an SQL() method that can be used directly by sql.DB.PrepareContext(ctx, prepared.SQL()).
• PreparedQuery has a Build method that accepts a map/struct argument, and returns a list of arguments ready to be used by db.Stmt.QueryContext returned from sql.DB.PrepareContext.
• It also has a Query method that accepts a map/struct argument, which returns a QueryWriter interface that can be used anywhere a QueryWriter is accepted, for example:

bob.All(context.Background(), bdb, prepared.Query(map[string]any{
	"offset": items[0],
	"limit":  items[1],
}), dataMapper)

The idea is to call Query.BuildPrepared(), and use it to create a Statement, and later on each DB call use PreparedQuery methods to map named parameters to the correct order and execute the statement, without needing the SQL again.

[RangelReale]

Did a third proof-of-concept that is much simpler, but relies on global functions to be able to set the named parameters and return values compatible with the other functions. As a bonus it also works with bob.Prepare, the other one would need a deeper integration.

[stephenafamo]

This 3rd proof of concept is looking closer to what I imagined it would look like. A few comments:

• Personally, I would prefer the name to be NamedArg instead of ArgNamed, and NamedArgGroup instead of ArgNamedGroup
• Because of how NamedArg/ArgNamed works, I don't think there's a need for NamedPlaceholder
• I haven't looked closely enough to understand the need to have a separate QueryWriter interface. Is there a way to avoid this?

[RangelReale]

Personally, I would prefer the name to be NamedArg instead of ArgNamed, and NamedArgGroup instead of ArgNamedGroup

Hmm this will conflict with the top level NamedArg function, I'm trying to think of a better name for it...

Because of how NamedArg/ArgNamed works, I don't think there's a need for NamedPlaceholder

I also didn't found a use-case for the Placeholder method, it adds variables but there's no way of knowing what order they will end up...

I haven't looked closely enough to understand the need to have a separate QueryWriter interface. Is there a way to avoid this?

It exists so here I don't need to implement the Expression interface also. Looking at it now, it is only an extra similar function, what is the difference between WriteQuery and WriteSQL?

[RangelReale]

WriteSQL is trivial to write, so I removed the QueryWriter interface.

[RangelReale]

I can open a PR using this branch if you want to follow this path, so we can comment there.

[stephenafamo]

Please do that. Even if we maybe don't merge it, it would be easier for conversations

[RangelReale]

#66

[stephenafamo]

Should be addressed by #298