feat: support unnamed parameters #3820

sakthivelmanii · 2025-04-14T05:29:42Z

Currently Spanner Client Library doesn't support passing positional (or unnamed) parameters while creating the statement. This feature provides the support for unnamed parameters which helps customers to create SQL Statement with unnamed parameter(?)

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

…nterface

google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java

...e-cloud-spanner/src/main/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClient.java

olavloite · 2025-04-14T07:26:42Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerTypeConverter.java

+  }
+
+  static ZonedDateTime convertToUTCTimezone(LocalDateTime localDateTime) {
+    return localDateTime.atZone(UTC_ZONE);


Would it not make more sense to interpret this value with the system default timezone?

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

olavloite · 2025-04-14T07:33:26Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

+    if (value instanceof java.util.Date) {
+      return Value.date(SpannerTypeConverter.convertUtilDateToSpannerDate((java.util.Date) value));
+    }


I'm not entirely sure about this conversion. java.util.Date is actually more like a TIMESTAMP than a DATE. We should consider:

Just disallowing the use of java.util.Date in the first place, as this class has serious flaws.

Or converting it to a TIMESTAMP rather than a DATE.

Removed java.util.Date

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

olavloite · 2025-04-14T07:37:41Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java

+    Statement statement = client.newStatementFactory().of("select id from test where b=?", true);
+    Statement generatedStatement =
+        Statement.newBuilder("select id from test where b=@p1").bind("p1").to(true).build();
+    mockSpanner.putStatementResult(StatementResult.query(generatedStatement, SELECT1_RESULTSET));


We should add tests for statements with:

Comments with question marks inside the comments

String literals with question marks inside the string literals

Statement hints at the start

olavloite · 2025-04-14T11:29:10Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java

@@ -41,6 +46,8 @@ class DatabaseClientImpl implements DatabaseClient {
  @VisibleForTesting final boolean useMultiplexedSessionPartitionedOps;
  @VisibleForTesting final boolean useMultiplexedSessionForRW;

+  private StatementFactory statementFactory = null;


nit: this can be removed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerTypeConverter.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

olavloite

LGTM, with some minor nits on javadoc, and missing null handling

olavloite · 2025-04-15T08:12:02Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClient.java

+  /**
+   * Returns StatementFactory for the given dialect.
+   *
+   * <p>A {@link StatementFactory}, can be used to create statements with unnamed parameters.
+   *
+   * <p>Examples using {@link StatementFactory}
+   *
+   * <p>databaseClient.getStatementFactory().of("SELECT NAME FROM TABLE WHERE ID = ?", 10)
+   */


Suggested change

/**

* Returns StatementFactory for the given dialect.

*

* A {@link StatementFactory}, can be used to create statements with unnamed parameters.

*

* Examples using {@link StatementFactory}

*

* databaseClient.getStatementFactory().of("SELECT NAME FROM TABLE WHERE ID = ?", 10)

*/

/**

* Returns a {@link StatementFactory} for the given dialect.

*

* A {@link StatementFactory} can be used to create statements with unnamed parameters.

* This is primarily intended for framework developers who want to integrate the Spanner client

* with frameworks that use unnamed parameters. Developers who just want to use the Spanner

* client in their application, should use named parameters.

*

* Examples using {@link StatementFactory}

*

* <pre>{@code

* Statement statement = databaseClient

* .getStatementFactory()

* .withUnnamedParameters("SELECT NAME FROM TABLE WHERE ID = ?", 10);

* }</pre>

*/

olavloite · 2025-04-15T08:16:40Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+  /**
+   * Factory for creating {@link Statement}.
+   *
+   * <p>This factory class supports creating {@link Statement} with positional(or unnamed)


nit:

Suggested change

* This factory class supports creating {@link Statement} with positional(or unnamed)

* This factory class supports creating {@link Statement} with positional (or unnamed)

olavloite · 2025-04-15T08:17:57Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+   *     .withUnnamedParameters("SELECT * FROM TABLE WHERE ID = ?", 10L)
+   * }</pre>
+   *
+   * How to use SQL queries with IN command


Suggested change

* How to use SQL queries with IN command

* How to use arrays with the IN operator

olavloite · 2025-04-15T08:18:48Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+   * Statement statement = databaseClient.getStatementFactory()
+   *     .withUnnamedParameters("SELECT * FROM TABLE WHERE ID = ?", 10L)
+   * }</pre>
+   *


Can we add an example here (so after the simple query, but before the array example) that shows how to use it with multiple parameters. This could for example be an INSERT statement that inserts a row with 2 or 3 column values.

olavloite · 2025-04-15T08:20:13Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

@@ -246,4 +248,90 @@ StringBuilder toString(StringBuilder b) {
    }
    return b;
  }
+
+  /**
+   * Factory for creating {@link Statement}.


Suggested change

* Factory for creating {@link Statement}.

* Factory for creating {@link Statement}s with unnamed parameters.

* This class is primarily intended for framework developers who want to integrate the Spanner

* client with a framework that uses unnamed parameters. Developers who want to use the

* Spanner client in their application, should use named parameters.

olavloite · 2025-04-15T08:22:33Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+     * <p>For Date column, following types are supported
+     *
+     * <ul>
+     *   <li>java.util.Date


Suggested change

* <li>java.util.Date

olavloite · 2025-04-15T08:22:43Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+     *
+     * <ul>
+     *   <li>java.util.Date
+     *   <li>LocalDate


Suggested change

* <li>LocalDate

* <li>java.time.LocalDate

olavloite · 2025-04-15T08:30:21Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+     * <p>For Timestamp column, following types are supported. All the dates should be in UTC
+     * format. Incase if the timezone is not in UTC, spanner client will convert that to UTC
+     * automatically


Suggested change

* For Timestamp column, following types are supported. All the dates should be in UTC

* format. Incase if the timezone is not in UTC, spanner client will convert that to UTC

* automatically

* For parameters of type TIMESTAMP, the following types are supported. Note that Spanner stores all

* timestamps in UTC. Instances of LocalDateTime and OffsetDateTime that use other timezones than UTC,

* will be converted to the corresponding UTC values before being sent to Spanner.

* Instances of LocalDateTime will be converted to a ZonedDateTime using the system default timezone,

* and then converted to UTC before being sent to Spanner.

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueTest.java

olavloite · 2025-04-15T11:43:47Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Statement.java

+   *    *     .withUnnamedParameters("INSERT INTO TABLE (ID, name, phonenumbers)
+   *       // VALUES(?, ?, ?)", id, name, phoneNumbers)


nit: this seems like a misformat / failed copy-paste

sakthivelmanii requested review from a team as code owners April 14, 2025 05:29

product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Apr 14, 2025

sakthivelmanii added 4 commits April 14, 2025 11:12

feat: support unnamed parameters

eebf2d0

Create staementfactory on-demand

29a5c5a

Remove unused variable

3f42072

Update clirr for exposing newStatementFactory in the DatabaseClient i…

899400c

…nterface

sakthivelmanii force-pushed the support_unnamed_parameters branch from 9aaf37b to 899400c Compare April 14, 2025 05:48

olavloite reviewed Apr 14, 2025

View reviewed changes

sakthivelmanii added 6 commits April 14, 2025 14:01

Address comments

d677799

Fix clirr issue with renaming

cf7b397

Addressed comments

fb618d5

Addressed comments

07e5569

Addressed comments

c8390b1

Added tests

668be97

olavloite reviewed Apr 14, 2025

View reviewed changes

Addressed comments

05713f3

olavloite approved these changes Apr 15, 2025

View reviewed changes

Addressed comments

0a7594c

olavloite approved these changes Apr 15, 2025

View reviewed changes

Addressed comments

1cd0c6e

sakthivelmanii merged commit 1afd815 into main Apr 16, 2025
34 checks passed

sakthivelmanii deleted the support_unnamed_parameters branch April 16, 2025 05:12

release-please bot mentioned this pull request Apr 15, 2025

chore(main): release 6.91.0 #3719

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support unnamed parameters #3820

feat: support unnamed parameters #3820

sakthivelmanii commented Apr 14, 2025 •

edited

Loading

olavloite Apr 14, 2025

sakthivelmanii Apr 15, 2025

olavloite Apr 14, 2025

sakthivelmanii Apr 14, 2025

olavloite Apr 14, 2025

sakthivelmanii Apr 15, 2025

olavloite Apr 14, 2025

sakthivelmanii Apr 15, 2025

olavloite left a comment

olavloite Apr 15, 2025

olavloite Apr 15, 2025

olavloite Apr 15, 2025

olavloite Apr 15, 2025

sakthivelmanii Apr 15, 2025

olavloite Apr 15, 2025

olavloite Apr 15, 2025

olavloite Apr 15, 2025

olavloite Apr 15, 2025

olavloite Apr 15, 2025

sakthivelmanii Apr 16, 2025

-  /**
-   * Returns StatementFactory for the given dialect.
-   *
-   * <p>A {@link StatementFactory}, can be used to create statements with unnamed parameters.
-   *
-   * <p>Examples using {@link StatementFactory}
-   *
-   * <p>databaseClient.getStatementFactory().of("SELECT NAME FROM TABLE WHERE ID = ?", 10)
-   */
+  /**
+   * Returns a {@link StatementFactory} for the given dialect.
+   *
+   * <p>A {@link StatementFactory} can be used to create statements with unnamed parameters.
+   * This is primarily intended for framework developers who want to integrate the Spanner client
+   * with frameworks that use unnamed parameters. Developers who just want to use the Spanner
+   * client in their application, should use named parameters.
+   *
+   * <p>Examples using {@link StatementFactory}
+   *
+   * <pre>{@code
+   * Statement statement = databaseClient
+   *     .getStatementFactory()
+   *     .withUnnamedParameters("SELECT NAME FROM TABLE WHERE ID = ?", 10);
+   * }</pre>
+   */

	* <p>This factory class supports creating {@link Statement} with positional(or unnamed)
	* <p>This factory class supports creating {@link Statement} with positional (or unnamed)

	* How to use SQL queries with IN command
	* How to use arrays with the IN operator

-   * Factory for creating {@link Statement}.
+   * Factory for creating {@link Statement}s with unnamed parameters.
+   * This class is primarily intended for framework developers who want to integrate the Spanner
+   * client with a framework that uses unnamed parameters. Developers who want to use the
+   * Spanner client in their application, should use named parameters.

-     * <p>For Timestamp column, following types are supported. All the dates should be in UTC
-     * format. Incase if the timezone is not in UTC, spanner client will convert that to UTC
-     * automatically
+     * <p>For parameters of type TIMESTAMP, the following types are supported. Note that Spanner stores all
+     * timestamps in UTC. Instances of LocalDateTime and OffsetDateTime that use other timezones than UTC,
+     * will be converted to the corresponding UTC values before being sent to Spanner.
+     * Instances of LocalDateTime will be converted to a ZonedDateTime using the system default timezone,
+     * and then converted to UTC before being sent to Spanner.

		* * .withUnnamedParameters("INSERT INTO TABLE (ID, name, phonenumbers)
		* // VALUES(?, ?, ?)", id, name, phoneNumbers)

feat: support unnamed parameters #3820

feat: support unnamed parameters #3820

Conversation

sakthivelmanii commented Apr 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olavloite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sakthivelmanii commented Apr 14, 2025 •

edited

Loading