fix: Clause implements Serializable to suport using from an Apache Beam pipline

[srfrnk]

I'd like to create a BEAM PR for the following:
srfrnk/beam@583c77c

However it has will require this PR for the driver to first propagate.

[tolbertam]

Thanks @srfrnk! This looks straightforward. I'll look at this tomorrow morning and also look at the rest of the query logger classes to see if anything else should be made serializable.

[tolbertam]

Sorry, it looks like I misunderstood when I made my original comment. I did not realize at the time that nothing in QueryBuilder is currently Serializable so it probably wouldn't be right to just serialize Clause.

For example BindMarker is not Serializable, so if you used Clause with a bindMarker, or any other values provided to the Clause are not Serializable then it is likely not to work. For that reason it probably is not a good idea to make QueryBuilder Serializable at this time.

[srfrnk]

Currently this prevents the Apache Beam CassandraIO from having a Where clause defined in the reader which makes every read injest the full table - a tremendous waste of resources!

To handle that I has to remove the code using your QueryBuilder to support a string Where clause.
I would rather use the QueryBuilder and pass a Clause but this requires it to be Serializeable.

@tolbertam is there anyway to handle that differently?

[tolbertam]

Is it more important that Clause is Serializable, or that you can use the QueryBuilder API to build your query from something that represents the clause part of the query?

If you wanted the capability of using the QueryBuilder API to build the rest of the query, you could implement your own custom Clause that wraps String, i.e.:

import com.datastax.driver.core.CodecRegistry;
import java.util.List;

public class StringClause extends Clause {

  private final String rawClause;

  public StringClause(String rawClause) {
    this.rawClause = rawClause;
  }

  @Override
  String name() {
    // only used for routing purposes
    return null;
  }

  @Override
  Object firstValue() {
    // only used for routing purposes
    return null;
  }

  @Override
  void appendTo(StringBuilder sb, List<Object> values, CodecRegistry codecRegistry) {
    sb.append(rawClause);
  }

  @Override
  boolean containsBindMarker() {
    return false;
  }
}

  private static String generateRangeQuery(
    String keyspace,
    String table,
    String where, // <-- change type back to String
    String partitionKey,
    BigInteger rangeStart,
    BigInteger rangeEnd)
  {
    Select.Where builder = QueryBuilder.select().from(keyspace,table).where();
    if(where!=null) {
      builder = builder.and(new StringClause(where)); // <-- use StringClause to wrap raw string
    }

    String token=String.format("token(%s)",partitionKey);

    if(rangeStart!=null) {
      builder = builder.and(QueryBuilder.gte(token, rangeStart));
    }

    if(rangeEnd!=null) {
      builder = builder.and(QueryBuilder.lt(token, rangeEnd));
    }

    String query = builder.toString();
    LOG.debug("Cassandra generated read query : {}", query);
    return query;
  }

This would allow you to build your full query using QueryBuilder by allowing the clause to be passed in as a raw string.

This could be generally useful enough that we add to the driver, but implementing StringClause yourself would allow you to make this work independently of a driver change.

[tolbertam]

Small caveat: that only works if StringClause is part of the com.datastax.driver.core.QueryBuilder package (because of weird visibility rules).

[srfrnk]

Yes this was my first idea but I ruled it out since implementing Clause requires visibility to internal types - which means it can't be done from Apache Beam code.
Also this seems to defeat the purpose as my intention to use Clause was to ensure safety. Once a String is allowed I can just as well build the entire query on my own.
Thanks anyway.

[adutra]

@srfrnk based on your last comment, are you OK if we close this PR? Thanks!

[srfrnk]

I will be doing the fixes in my own fork and use this anyway in my code from now on so you can close this if you want.

[adutra]

Thanks!