fix: support repeated named parameters

[ayufan]

This fixes got %v argument values, but found %v parameters in the sql string error when using repeated named arguments.

The "workaround" is to specify as many parameters as observed. The last one will win due to prepareSpannerStmt using map keyed by name. The fix will make the Bad examples to return an error, while Good will start to work.

Example:

// Bad: @name will be "value2"
s.db.QueryRow(
  "SELECT * FROM users WHERE (first_name=@name OR last_name=@name)",
  sql.Named("name", "value"),
  sql.Named("name", "value2"),
)

// Bad: @name will be "value2"
s.db.QueryRow(
  "SELECT * FROM users WHERE (first_name=@name OR last_name=@name)",
  "value",
  "value2",
)

// Good: works, ideal usage
s.db.QueryRow(
  "SELECT * FROM users WHERE (first_name=@name OR last_name=@name)",
  sql.Named("name", "value"),
)

// Good: not ideal, inferred positional arguments
s.db.QueryRow(
  "SELECT * FROM users WHERE (first_name=@name OR last_name=@name)",
  "value",
)

Disclaimer

GitLab is working on a new project Topology Service that plans to use Cloud Spanner. This has been an identified bug and part of effort to upstream fixes. Proof of concept is not public, below are public materials available:

• https://gitlab.com/gitlab-org/cells/topology-service/-/issues/3
• https://docs.gitlab.com/ee/architecture/blueprints/cells/topology_service.html

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[olavloite]

@ayufan Thanks for the contribution. While I agree that the proposed behavior would probably have been the best from the start, changing it in the way that this pull request does is a breaking change for anyone who currently uses any of the workarounds that are marked as 'bad' in the description, as it goes from working (albeit not in a great way), into returning an error.

Let me see if there is a combination that retains the current behavior, but also fixes it for the more logical use case (especially the one that uses named parameters).

[olavloite]

@ayufan We've created an alternative solution in #240 that retains the current behavior, but also adds support for re-using named parameters. It also updates the documentation to clarify what the recommended usage is of named vs positional parameters.

We are therefore going to close this pull request, but I would still like to thank you for pointing out this issue in the driver, and for the proposed fix.

[ayufan]

Thank you @olavloite for swift fix. I'm happy with a "balanced" solution for this.