Fair points. I can make those changes if that's a concern. -- Michael Mior [email protected]
2016-04-22 12:00 GMT-04:00 Julian Hyde <[email protected]>: > 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() { > > > >
