Conditional DELETE from a table with a DATE column fails

[O1O1O1O]

Thanks for stopping by to let us know something could be better!

Please include as much information as possible:

Environment details

• OS: N/A
• Java version: 1.8.0_162
• google-cloud-java version(s): google-cloud-spanner 0.57.0-beta

Steps to reproduce

1. Create table with schema including a DATE column eg.

CREATE TABLE geo_views (
	city STRING(MAX) NOT NULL,
	client_id INT64 NOT NULL,
	date DATE NOT NULL,
	domain STRING(MAX) NOT NULL,
	url STRING(MAX) NOT NULL,
	pageviews INT64 NOT NULL,
) PRIMARY KEY (client_id, domain, date DESC, city, url)

2. Add some records.
3. Delete from the table with a WHERE clause eg. DELETE FROM geo_views WHERE client_id > 1000

Stacktrace

Note this stacktrace comes from a delete when using the Olaviate Spanner JDBC driver which includes the latest Google Spanner client code as a shaded lib:

java.lang.AssertionError: Illegal key part: class nl.topicus.jdbc.shaded.com.google.cloud.Date
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.Key.toProto(Key.java:287)
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.KeySet.appendToProto(KeySet.java:204)
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.Mutation.toProto(Mutation.java:381)
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.SpannerImpl$TransactionContextImpl.commit(SpannerImpl.java:1394)
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.SpannerImpl$TransactionRunnerImpl.runInternal(SpannerImpl.java:1299)
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.SpannerImpl$TransactionRunnerImpl.run(SpannerImpl.java:1242)
	at nl.topicus.jdbc.shaded.com.google.cloud.spanner.SessionPool$PooledSession$1.run(SessionPool.java:398)
	at nl.topicus.jdbc.transaction.TransactionThread.run(TransactionThread.java:115)

Code snippet

I believe this error is caused by lack of support for the Date type in the if statement here: https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Key.java#L285

  ListValue toProto() {
    ListValue.Builder builder = ListValue.newBuilder();
    for (Object part : parts) {
      if (part == null) {
        builder.addValues(NULL_PROTO);
      } else if (part instanceof Boolean) {
        builder.addValuesBuilder().setBoolValue((Boolean) part);
      } else if (part instanceof Long) {
        builder.addValuesBuilder().setStringValue(part.toString());
      } else if (part instanceof Double) {
        builder.addValuesBuilder().setNumberValue((Double) part);
      } else if (part instanceof String) {
        builder.addValuesBuilder().setStringValue((String) part);
      } else if (part instanceof ByteArray) {
        builder.addValuesBuilder().setStringValue(((ByteArray) part).toBase64());
      } else if (part instanceof Timestamp) {
        builder.addValuesBuilder().setStringValue(((Timestamp) part).toString());
      } else {
        throw new AssertionError("Illegal key part: " + part.getClass());
      }
    }
    return builder.build();
  }

External references such as API reference guides used

See also: olavloite/spanner-jdbc#134

Any additional information below

I looked in the tests for this code but there aren't any because Key.toProto is not public and only used by higher level code in the client package. As best I can tell only an actual delete using a key based condition will trigger its use.

[O1O1O1O]

Thanks for the rapid turn around on fixing this @sduskis 🥇