Nice work, Michael. The only things I’d have done differently are to rename the test case method from testFilterUUID to testFilterUuid (yes, we camel-case acronyms), and to reference the JIRA case in a javadoc comment against that method.
> On Apr 22, 2016, at 6:30 AM, [email protected] wrote: > > Repository: calcite > Updated Branches: > refs/heads/master dd8d3c588 -> c07f33ae6 > > > [CALCITE-1210] Allow UUID filtering in Cassandra > > > Project: http://git-wip-us.apache.org/repos/asf/calcite/repo > Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/c07f33ae > Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/c07f33ae > Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/c07f33ae > > Branch: refs/heads/master > Commit: c07f33ae62fd7b3c91713e3f7a038031fb7db5b9 > Parents: dd8d3c5 > Author: Michael Mior <[email protected]> > Authored: Fri Apr 22 00:52:12 2016 -0400 > Committer: Michael Mior <[email protected]> > Committed: Fri Apr 22 09:24:25 2016 -0400 > > ---------------------------------------------------------------------- > .../adapter/cassandra/CassandraFilter.java | 17 ++++++++++++----- > .../adapter/cassandra/CassandraSchema.java | 8 ++++++-- > .../apache/calcite/test/CassandraAdapterIT.java | 19 ++++++++++++++++--- > 3 files changed, 34 insertions(+), 10 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/calcite/blob/c07f33ae/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java > ---------------------------------------------------------------------- > diff --git > a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java > > b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java > index 41eb3ad..ba8aa9c 100644 > --- > a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java > +++ > b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraFilter.java > @@ -27,10 +27,12 @@ import org.apache.calcite.rel.RelFieldCollation; > import org.apache.calcite.rel.RelNode; > import org.apache.calcite.rel.core.Filter; > import org.apache.calcite.rel.metadata.RelMetadataQuery; > +import org.apache.calcite.rel.type.RelDataType; > import org.apache.calcite.rex.RexCall; > import org.apache.calcite.rex.RexInputRef; > import org.apache.calcite.rex.RexLiteral; > import org.apache.calcite.rex.RexNode; > +import org.apache.calcite.sql.type.SqlTypeName; > import org.apache.calcite.util.Util; > > import java.util.ArrayList; > @@ -67,8 +69,8 @@ public class CassandraFilter extends Filter implements > CassandraRel { > this.implicitFieldCollations = implicitFieldCollations; > > Translator translator = > - new Translator(CassandraRules.cassandraFieldNames(getRowType()), > - partitionKeys, clusteringKeys, implicitFieldCollations); > + new Translator(getRowType(), partitionKeys, clusteringKeys, > + implicitFieldCollations); > this.match = translator.translateMatch(condition); > this.singlePartition = translator.isSinglePartition(); > this.implicitCollation = translator.getImplicitCollation(); > @@ -111,15 +113,17 @@ public class CassandraFilter extends Filter implements > CassandraRel { > > /** Translates {@link RexNode} expressions into Cassandra expression > strings. */ > static class Translator { > + private final RelDataType rowType; > private final List<String> fieldNames; > private final Set<String> partitionKeys; > private final List<String> clusteringKeys; > private int restrictedClusteringKeys; > private final List<RelFieldCollation> implicitFieldCollations; > > - Translator(List<String> fieldNames, List<String> partitionKeys, > List<String> clusteringKeys, > + Translator(RelDataType rowType, List<String> partitionKeys, List<String> > clusteringKeys, > List<RelFieldCollation> implicitFieldCollations) { > - this.fieldNames = fieldNames; > + this.rowType = rowType; > + this.fieldNames = CassandraRules.cassandraFieldNames(rowType); > this.partitionKeys = new HashSet<String>(partitionKeys); > this.clusteringKeys = clusteringKeys; > this.restrictedClusteringKeys = 0; > @@ -267,7 +271,10 @@ public class CassandraFilter extends Filter implements > CassandraRel { > Object value = literalValue(right); > String valueString = value.toString(); > if (value instanceof String) { > - valueString = "'" + valueString + "'"; > + SqlTypeName typeName = rowType.getField(name, true, > false).getType().getSqlTypeName(); > + if (typeName != SqlTypeName.CHAR) { > + valueString = "'" + valueString + "'"; > + } > } > return name + " " + op + " " + valueString; > } > > http://git-wip-us.apache.org/repos/asf/calcite/blob/c07f33ae/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java > ---------------------------------------------------------------------- > diff --git > a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java > > b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java > index 47312bf..2cbc342 100644 > --- > a/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java > +++ > b/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java > @@ -120,9 +120,13 @@ public class CassandraSchema extends AbstractSchema { > > // TODO: This mapping of types can be done much better > SqlTypeName typeName = SqlTypeName.ANY; > - if (type == DataType.ascii() || type == DataType.text() || type == > DataType.varchar() > - || type == DataType.uuid() || type == DataType.timeuuid()) { > + if (type == DataType.uuid() || type == DataType.timeuuid()) { > + // We currently rely on this in CassandraFilter to detect UUID > columns. > + // That is, these fixed length literals should be unquoted in CQL. > typeName = SqlTypeName.CHAR; > + } else if (type == DataType.ascii() || type == DataType.text() > + || type == DataType.varchar()) { > + typeName = SqlTypeName.VARCHAR; > } else if (type == DataType.cint() || type == DataType.varint()) { > typeName = SqlTypeName.INTEGER; > } else if (type == DataType.bigint()) { > > http://git-wip-us.apache.org/repos/asf/calcite/blob/c07f33ae/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterIT.java > ---------------------------------------------------------------------- > diff --git > a/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterIT.java > b/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterIT.java > index 8cf6673..c0e92a9 100644 > --- a/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterIT.java > +++ b/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterIT.java > @@ -71,10 +71,23 @@ public class CassandraAdapterIT { > .returns("username=!PUBLIC!; > time=e8754000-80b8-1fe9-8e73-e3698c967ddd; " > + "tweet_id=f3c329de-d05b-11e5-b58b-90e2ba530b12\n") > .explainContains("PLAN=CassandraToEnumerableConverter\n" > - + " CassandraFilter(condition=[=(CAST($0):CHAR(8) CHARACTER SET > \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", '!PUBLIC!')])\n" > + + " CassandraFilter(condition=[=(CAST($0):VARCHAR(8) CHARACTER > SET \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", '!PUBLIC!')])\n" > + " CassandraTableScan(table=[[twissandra, userline]]"); > } > > + @Test public void testFilterUUID() { > + CalciteAssert.that() > + .enable(enabled()) > + .with(TWISSANDRA) > + .query("select * from \"tweets\" where > \"tweet_id\"='f3cd759c-d05b-11e5-b58b-90e2ba530b12'") > + .limit(1) > + .returns("tweet_id=f3cd759c-d05b-11e5-b58b-90e2ba530b12; " > + + "body=Lacus augue pede posuere.; username=JmuhsAaMdw\n") > + .explainContains("PLAN=CassandraToEnumerableConverter\n" > + + " CassandraFilter(condition=[=(CAST($0):CHAR(36) CHARACTER SET > \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", > 'f3cd759c-d05b-11e5-b58b-90e2ba530b12')])\n" > + + " CassandraTableScan(table=[[twissandra, tweets]]"); > + } > + > @Test public void testSort() { > CalciteAssert.that() > .enable(enabled()) > @@ -83,7 +96,7 @@ public class CassandraAdapterIT { > .returnsCount(146) > .explainContains("PLAN=CassandraToEnumerableConverter\n" > + " CassandraSort(sort0=[$1], dir0=[DESC])\n" > - + " CassandraFilter(condition=[=(CAST($0):CHAR(8) CHARACTER > SET \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", '!PUBLIC!')])\n"); > + + " CassandraFilter(condition=[=(CAST($0):VARCHAR(8) > CHARACTER SET \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", > '!PUBLIC!')])\n"); > } > > @Test public void testProject() { > @@ -95,7 +108,7 @@ public class CassandraAdapterIT { > .explainContains("PLAN=CassandraToEnumerableConverter\n" > + " CassandraProject(tweet_id=[$2])\n" > + " CassandraSort(fetch=[1])\n" > - + " CassandraFilter(condition=[=(CAST($0):CHAR(8) > CHARACTER SET \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", > '!PUBLIC!')])\n"); > + + " CassandraFilter(condition=[=(CAST($0):VARCHAR(8) > CHARACTER SET \"ISO-8859-1\" COLLATE \"ISO-8859-1$en_US$primary\", > '!PUBLIC!')])\n"); > } > > @Test public void testMaterializedView() { >
