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

back: Investigate the need for db.Prepare usage and add issues as needed #74

Closed
daved opened this issue Nov 27, 2019 · 4 comments
Closed
Assignees

Comments

@daved
Copy link
Member

daved commented Nov 27, 2019

For example, in back/internal/usersvc/internal/userdb/qryuser.go

@daved daved changed the title Investigate the need for db.Prepare usage and add issues as needed back: Investigate the need for db.Prepare usage and add issues as needed Nov 27, 2019
@daved
Copy link
Member Author

daved commented May 7, 2020

AC

  • Find and read article(s) discussing how the Go SQL libraries use Prepare
    • Post links in this issue and "at" Daved.
  • If any new issues are noticed, please create them.
  • Daved should review current SQL usage and create issues as noticed.

@codegold79 codegold79 self-assigned this May 8, 2020
@codegold79
Copy link
Collaborator

codegold79 commented May 8, 2020

The idiomatic ways to form queries are listed in the Go Database/SQL Tutorial.

There are several idiomatic operations to retrieve results from the datastore.

  1. Execute a query that returns rows.
  2. Prepare a statement for repeated use, execute it multiple times, and destroy it.
  3. Execute a statement in a once-off fashion, without preparing it for repeated use.
  4. Execute a query that returns a single row. There is a shortcut for this special case.

I have made queries that are used once then closed. That means the second way (using db.Prepare()) is overkill. I do execute queries that only returns rows, so the first way is probably how I should be writing queries, like the Go SQL tutorial example,

rows, err := db.Query("select id, name from users where id = ?", 1)

I use the third case to set the deleted_at field where I don't care about the returned rows. The example given by the Go SQL tutorial was

_, err := db.Exec("DELETE FROM users") // OK

As of PR #83, I am using the following which seems fine:

	_, err := s.db.Exec(
		"UPDATE user SET deleted_at = ? WHERE user_id = ?",
		time.Now(),
		sid,
	)

I also use the fourth case where queries return single rows. Those queries would use a shortcut:

err = db.QueryRow("select name from users where id = ?", 1).Scan(&name)

@daved I believe I'm overusing the db.Prepare() method of querying the database and since I'm not using these queries multiple times, I should replace them all with db.Query(), db.Exec(), or db.QueryRow(). I should not use db.Prepare().

If you agree, I can make a new issue to get those queries converted.

@daved
Copy link
Member Author

daved commented May 12, 2020

Please review http://go-database-sql.org/prepared.html#avoiding-prepared-statements, along with the linked article (https://orangematter.solarwinds.com/2014/11/19/analyzing-prepared-statement-performance/), and ping me via Slack to setup a chat to discuss this further.

@codegold79
Copy link
Collaborator

I've read the section on "Avoiding Prepared Statements" in go-database-sql.org several times. I realized during my last reading that db.Prepare is a confusing name. To me, it seemed to imply that db.Query does not prepare statements. However, from that section, db.Query does also prepare statements:

Go creates prepared statements for you under the covers. A simple db.Query(sql, param1, param2), for example, works by preparing the sql, then executing it with the parameters and finally closing the statement.

So, how does one execute a query without preparing the statements? It requires more steps:

If you don’t want to use a prepared statement, you need to use fmt.Sprint() or similar to assemble the SQL, and pass this as the only argument to db.Query() or db.QueryRow(). And your driver needs to support plaintext query execution, which is added in Go 1.1 via the Execer and Queryer interfaces, documented here.

The second article at orangematter.solarwinds.com says there's a case for taking the extra steps the write plaintext query executions:

If we change from a prepared statement to a plaintext query here, we’ll reduce round-trips on the network and free up a lot of resources in the database server. Simple queries like this don’t benefit from being prepared. We’ll handle the security aspect of this query by validating input (which our API framework already does for us).

The simple queries in question were of the form SELECT shard_hostname FROM customer.env WHERE id = ?.

The trade off here is reduction to database load vs. writing more lines of code to avoiding preparing statements. And even if we did write extra code, will it work? Does our Go sql driver even support plaintext query execution?

I'll send a ping on Slack to discuss further.

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

No branches or pull requests

2 participants