Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Named arguments #66

Closed
wants to merge 9 commits into from
Closed

Conversation

RangelReale
Copy link
Contributor

Creates named arguments at generation time and allows replacements using a map/struct.

@@ -0,0 +1,196 @@
package main
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is temporary for showing the usage, will be removed in the final version.

Comment on lines +21 to +23
if hasNonNamed {
return nil, fmt.Errorf("cannot mix named and non-named arguments")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tecnically it wouldn't be a problem mixing named and non-named parameters, I could just forward the parameter in the same order if it wasn't a NamedArgument.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how will that work with prepared statements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mergeNamedArguments would receive []any, and in this for loop, I would check if the type is NamedArgument, otherwise I would just add the value itself to the return array at the same position.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example, given:

	query := psql.Insert(
		im.Into("actor", "first_name", "last_name"),
                im.Values(psql.Arg(12, psql.NamedArg("in"))),
	)

mergeNamedArguments would receive as first parameter:

[]any{12, NamedArg{"in"}}

in the for loop I would just forward the value that is not NamedArg and only replace the other one, so in:

	queryStr, args, err := bob.BuildWithNamedArgs(query, map[string]any{
		"in": "LAST_NAME",
	})

args would be []any{12, "LAST_NAME"}.

Comment on lines +65 to +67
func ArgNamed(names ...string) Expression {
return bmod.ArgNamed(names...)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the discussion you talked about calling this NamedArg, but there is a NamedArg function already in this scope, maybe I could call that one Named, which would more or less resemble the sql.Named function which is for named arguments also?

@RangelReale
Copy link
Contributor Author

One thing that I'm not entirelly sure is the "Named args" naming, I think it may confuse users about SQL servers that do support named parameters.

Maybe we could call it "Bind" or "Bound" instead of "Named"? BoundArgument, BindArg, etc.

@RangelReale
Copy link
Contributor Author

I did a quick test using "Bind" or "Binding" instead of "named arg", and I think it made the intent much clearer.
RangelReale:bind-args-v3

With this naming it was also possible to make it much simpler, instead of new top-level functions, a single BindQuery toplevel function which returns an interface that extends Query and also has the 4 Build functions made it work similar to what is done currently.

Also adding a .Bind method to BaseQuery made it much easier to use, for example:

	queryStr, args, err := query.Bind(map[string]any{
		"in1": 15,
		"in2": "LAST_NAME",
	}).Build()

@stephenafamo
Copy link
Owner

See #104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants