blerer commented on code in PR #3301:
URL: https://github.com/apache/cassandra/pull/3301#discussion_r1607788871
##########
src/antlr/Parser.g:
##########
@@ -1797,6 +1811,8 @@ relation[WhereClause.Builder clauses]
}
| type=relationType tupleMarker=markerForTuple /* (a, b, c) >= ? */
{ $clauses.add(Relation.multiColumn(ids, type, tupleMarker)); }
+ | K_BETWEEN literals=betweenLiterals
+ { $clauses.add(Relation.multiColumn(ids, Operator.BETWEEN,
literals)); }
Review Comment:
The current change support literals (`BETWEEN 1 AND 2`) but not markers
(`BETWEEN ? AND ?`).
##########
src/java/org/apache/cassandra/cql3/Relation.java:
##########
@@ -114,6 +115,7 @@ public static Relation mapElement(ColumnIdentifier
identifier, Term.Raw rawKey,
public static Relation multiColumn(List<ColumnIdentifier> identifiers,
Operator operator, Term.Raw rawTerm)
{
assert operator != Operator.IN;
+ assert operator != Operator.BETWEEN;
Review Comment:
you can combine them in a single assertion `assert operator != Operator.IN
&& operator != Operator.BETWEEN;`
##########
src/antlr/Parser.g:
##########
@@ -1768,6 +1780,8 @@ relationType returns [Operator op]
relation[WhereClause.Builder clauses]
: name=cident type=relationType t=term {
$clauses.add(Relation.singleColumn(name, type, t)); }
+ | name=cident K_BETWEEN betweenValues=singleColumnBetweenValues
Review Comment:
an alternative could be
`| name=cident K_BETWEEN t1=term K_AND t2=term {
$clauses.add(Relation.singleColumn($name.id, Operator.BETWEEN,
Arrays.asList(t1, t2)); } `
##########
src/java/org/apache/cassandra/cql3/Operator.java:
##########
@@ -266,6 +266,59 @@ public boolean canBeUsedWith(ColumnsExpression.Kind kind)
return kind != ColumnsExpression.Kind.MAP_ELEMENT;
}
},
+ BETWEEN(16)
Review Comment:
What you say make sense. There is a separation between the serialization
value of an Operator and the enum order. Now, the issue is that people will
pick the last number of the list and use the next one for the serialization of
the next enum they will add. It is not perfect but I will still put between at
the end to avoid this issue that can make people lose time trying to figure out
what is going on when they run into an issue with the tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
