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

Implement named args for prepared statements #71

Closed
wants to merge 7 commits into from

Conversation

stephenafamo
Copy link
Owner

@RangelReale I finally have some time to take a look at implementing named binds.

In my implementation, named arguments can only be used with prepared statements.
This is because being able to have named arguments is primarily for ease of reusing queries, and that is what prepared statements should be for.

Here is what I have so far:

// How it will be used

func main1(ctx context.Context, exec bob.Preparer) {
	query := psql.Select(
		sm.From("users"),
		sm.Where(psql.Quote("id").In(bob.NamedGroup("in1", "in2", "in3"))),
		sm.Where(psql.Raw("id >= ?", bob.Named("id1"))),
	)

	// USING A map[string]any
	mappedStmt, err := bob.PrepareMapped(ctx, exec, query)
	if err != nil {
		panic(err)
	}

	_, err = mappedStmt.Exec(ctx, map[string]any{
		"id1": 400,
		"in1": 15,
		"in2": 200,
		"in3": 300,
	})
	if err != nil {
		panic(err)
	}

	// USING A struct
	type vals struct {
		ID1 int
		In1 int
		In2 int
		In3 int
	}

	boundStmt, err := bob.PrepareBound[vals](ctx, exec, query)
	if err != nil {
		panic(err)
	}

	_, err = boundStmt.Exec(ctx, vals{
		ID1: 400,
		In1: 15,
		In2: 200,
		In3: 300,
	})
	if err != nil {
		panic(err)
	}
}
SELECT
*
FROM users
WHERE ("id" IN ($1, $2, $3)) AND (id >= $4)

0: 15
1: 200
2: 300
3: 400


SELECT
*
FROM users
WHERE ("id" IN ($1, $2, $3)) AND (id >= $4)

0: 15
1: 200
2: 300
3: 400

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 10, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: dc037bf
Status: ✅  Deploy successful!
Preview URL: https://9ef19255.bob-8vc.pages.dev
Branch Preview URL: https://feature-prepare-named-bind.bob-8vc.pages.dev

View logs

@RangelReale
Copy link
Contributor

I'll take a look today, but don't you think that using Named() may confuse users with databases that have native named arguments?

@stephenafamo
Copy link
Owner Author

I'll take a look today, but don't you think that using Named() may confuse users with databases that have native named arguments?

It may, but I could not think of a different name to use.

  • NamedArg would suffer from the same confusion since database/sql has Named and NamedArg
  • Bind is often used to bind query results to objects
  • NamedPlaceholder would be the most accurate, but it looks too long.

@RangelReale
Copy link
Contributor

I was thinking maybe BoundArg, or BindArg, but it is not perfect also.

@RangelReale
Copy link
Contributor

One problem that I found is that I need to have a way to make tx.Stmt work, when using transactions we call DB.Prepare to create an Stmt, then when running in a transaction I must convert the Stmt to a transaction-specific one, but for this strongly-typed Stmt, we will need some way to wrap this statement.

@RangelReale
Copy link
Contributor

The statement storage declaration also is not very readable, for some reason I had to declare all generics parameters, and need to know beforehand which type each query would use, including the case of the struct is used as the parameter.

type storage struct {
	preparedSelectStatement         *bob.MappedQueryStmt[entity.Role, []entity.Role]
	preparedSelectByRoleIDStatement *bob.MappedQueryStmt[entity.Role, []entity.Role]
	preparedDeleteByRoleIDStatement *bob.MappedStmt
	preparedUpdateStatement         *bob.BoundQueryStmt[entity.Role, entity.Role, []entity.Role]
	preparedInsertStatement         *bob.BoundQueryStmt[entity.Role, entity.Role, []entity.Role]
	preparedSelectPolicyStatement   *bob.MappedQueryStmt[entity.RolePolicy, []entity.RolePolicy]
	preparedDeletePolicyStatement   *bob.MappedStmt
}

@stephenafamo
Copy link
Owner Author

One problem that I found is that I need to have a way to make tx.Stmt work, when using transactions we call DB.Prepare to create an Stmt, then when running in a transaction I must convert the Stmt to a transaction-specific one, but for this strongly-typed Stmt, we will need some way to wrap this statement.

I had not thought of this before. This definitely has to be made easier.

The statement storage declaration also is not very readable, for some reason I had to declare all generics parameters, and need to know beforehand which type each query would use, including the case of the struct is used as the parameter.

This is as-designed.

  • The Statements need to know the type to scan results into
  • The struct-bound statements also need to know the struct to expect as an arg.

While this feels limiting, when using it anywhere in the code, it gives a certain level of confidence that the query will work as expected.
It could be less strict, but I kind of like it this way.

@stephenafamo
Copy link
Owner Author

One problem that I found is that I need to have a way to make tx.Stmt work, when using transactions we call DB.Prepare to create an Stmt, then when running in a transaction I must convert the Stmt to a transaction-specific one, but for this strongly-typed Stmt, we will need some way to wrap this statement.

I have added InTx methods to the Statements to make it possible to use create transaction-specific variants. Try it out and let me know.

@RangelReale
Copy link
Contributor

One problem that I found is that I need to have a way to make tx.Stmt work, when using transactions we call DB.Prepare to create an Stmt, then when running in a transaction I must convert the Stmt to a transaction-specific one, but for this strongly-typed Stmt, we will need some way to wrap this statement.

I have added InTx methods to the Statements to make it possible to use create transaction-specific variants. Try it out and let me know.

I think it should work, I was having some difficulties because we use some repository transaction pattern so I didn't have an *sql.Tx instance available, but the interface wasn't really good anyway.

@RangelReale
Copy link
Contributor

I'm still not sure about the amount of complexity added just to be able to set the parameter values after the query generation. It is now not possible to use only the query builder for prepared queries, you are required to use the library Stmt type and Prepare calls, which ties it to a specific DB implementation.

@stephenafamo
Copy link
Owner Author

See #104

@stephenafamo stephenafamo deleted the feature/prepare-named-bind branch January 2, 2024 00:36
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.

2 participants