Maxwell-Guo commented on code in PR #2655:
URL: https://github.com/apache/cassandra/pull/2655#discussion_r1378849571
##########
src/java/org/apache/cassandra/cql3/SingleColumnRelation.java:
##########
@@ -181,35 +177,35 @@ public boolean equals(Object o)
@Override
protected Restriction newEQRestriction(TableMetadata table,
VariableSpecifications boundNames)
{
- ColumnMetadata columnDef = table.getExistingColumn(entity);
+ ColumnMetadata column = table.getExistingColumn(entity);
+ validateReceiver(column);
if (mapKey == null)
{
- Term term = toTerm(toReceivers(columnDef), value, table.keyspace,
boundNames);
- return new SingleColumnRestriction.EQRestriction(columnDef, term);
+ Term term = toTerm(column, value, table.keyspace, boundNames);
+ return new SingleColumnRestriction.EQRestriction(column, term);
}
- List<? extends ColumnSpecification> receivers = toReceivers(columnDef);
- Term entryKey = toTerm(Collections.singletonList(receivers.get(0)),
mapKey, table.keyspace, boundNames);
- Term entryValue = toTerm(Collections.singletonList(receivers.get(1)),
value, table.keyspace, boundNames);
- return new SingleColumnRestriction.ContainsRestriction(columnDef,
entryKey, entryValue);
+
+ checkFalse(column.type instanceof ListType, "Indexes on list entries
(%s[index] = value) are not currently supported.", column.name);
Review Comment:
I think the word "currently" can be removed to keep align with "Map-entry
equality predicates on frozen map column %s are not supported",
##########
src/java/org/apache/cassandra/cql3/SingleColumnRelation.java:
##########
@@ -181,35 +177,35 @@ public boolean equals(Object o)
@Override
protected Restriction newEQRestriction(TableMetadata table,
VariableSpecifications boundNames)
{
- ColumnMetadata columnDef = table.getExistingColumn(entity);
+ ColumnMetadata column = table.getExistingColumn(entity);
+ validateReceiver(column);
if (mapKey == null)
{
- Term term = toTerm(toReceivers(columnDef), value, table.keyspace,
boundNames);
- return new SingleColumnRestriction.EQRestriction(columnDef, term);
+ Term term = toTerm(column, value, table.keyspace, boundNames);
+ return new SingleColumnRestriction.EQRestriction(column, term);
}
- List<? extends ColumnSpecification> receivers = toReceivers(columnDef);
- Term entryKey = toTerm(Collections.singletonList(receivers.get(0)),
mapKey, table.keyspace, boundNames);
- Term entryValue = toTerm(Collections.singletonList(receivers.get(1)),
value, table.keyspace, boundNames);
- return new SingleColumnRestriction.ContainsRestriction(columnDef,
entryKey, entryValue);
+
+ checkFalse(column.type instanceof ListType, "Indexes on list entries
(%s[index] = value) are not currently supported.", column.name);
+ checkTrue(column.type instanceof MapType, "Column %s cannot be used as
a map", column.name);
+ checkTrue(column.type.isMultiCell(), "Map-entry equality predicates on
frozen map column %s are not supported", column.name);
+
+ Term entryKey = toTerm(makeCollectionReceiver(column, true), mapKey,
table.keyspace, boundNames);
+ Term entryValue = toTerm(makeCollectionReceiver(column, false), value,
table.keyspace, boundNames);
+ return new SingleColumnRestriction.ContainsRestriction(column,
entryKey, entryValue);
}
@Override
protected Restriction newINRestriction(TableMetadata table,
VariableSpecifications boundNames)
{
- ColumnMetadata columnDef = table.getExistingColumn(entity);
- List<? extends ColumnSpecification> receivers = toReceivers(columnDef);
- List<Term> terms = toTerms(receivers, inValues, table.keyspace,
boundNames);
- if (terms == null)
- {
- Term term = toTerm(receivers, value, table.keyspace, boundNames);
- return new
SingleColumnRestriction.InRestrictionWithMarker(columnDef, (Lists.Marker) term);
- }
+ ColumnMetadata column = table.getExistingColumn(entity);
+ validateReceiver(column);
+ Terms terms = toTerms(column, inValues, table.keyspace, boundNames);
- // An IN restrictions with only one element is the same than an EQ
restriction
- if (terms.size() == 1)
- return new SingleColumnRestriction.EQRestriction(columnDef,
terms.get(0));
+ // An IN restrictions with only one element is the same as an EQ
restriction
Review Comment:
restrictions -> restriction ?
##########
src/java/org/apache/cassandra/cql3/Terms.java:
##########
@@ -47,225 +47,515 @@ public Object get(int index)
@Override
public int size()
{
- return 0;
+ throw new UnsupportedOperationException();
+ }
+ };
+
+ Terminals UNSET_TERMINALS = new Terminals()
+ {
+ @Override
+ @SuppressWarnings("unchecked")
+ public List<ByteBuffer> get()
+ {
+ return (List<ByteBuffer>) UNSET_LIST;
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public List<List<ByteBuffer>> getElements()
+ {
+ return (List<List<ByteBuffer>>) UNSET_LIST;
+ }
+
+ @Override
+ public List<Terminal> asList()
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void addFunctionsTo(List<Function> functions)
+ {
+
Review Comment:
this function is left with empty implementation ?
##########
src/java/org/apache/cassandra/cql3/Terms.java:
##########
@@ -47,225 +47,515 @@ public Object get(int index)
@Override
public int size()
{
- return 0;
+ throw new UnsupportedOperationException();
+ }
+ };
+
+ Terminals UNSET_TERMINALS = new Terminals()
+ {
+ @Override
+ @SuppressWarnings("unchecked")
+ public List<ByteBuffer> get()
+ {
+ return (List<ByteBuffer>) UNSET_LIST;
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public List<List<ByteBuffer>> getElements()
+ {
+ return (List<List<ByteBuffer>>) UNSET_LIST;
+ }
+
+ @Override
+ public List<Terminal> asList()
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void addFunctionsTo(List<Function> functions)
+ {
+
+ }
+
+ @Override
+ public boolean containsSingleTerm()
+ {
+ return false;
+ }
+
+ @Override
+ public Term asSingleTerm()
+ {
+ throw new UnsupportedOperationException();
}
};
/**
* Adds all functions (native and user-defined) used by any of the terms
to the specified list.
* @param functions the list to add to
*/
- public void addFunctionsTo(List<Function> functions);
+ void addFunctionsTo(List<Function> functions);
/**
* Collects the column specifications for the bind variables in the terms.
- * This is obviously a no-op if the terms are Terminal.
+ * This is obviously a no-op if the terms are Terminals.
*
* @param boundNames the variables specification where to collect the
* bind variables of the terms in.
*/
- public void collectMarkerSpecification(VariableSpecifications boundNames);
+ void collectMarkerSpecification(VariableSpecifications boundNames);
/**
* Bind the values in these terms to the values contained in {@code
options}.
- * This is obviously a no-op if the term is Terminal.
+ * This is obviously a no-op if this {@code Terms} are Terminals.
*
- * @param options the values to bind markers to.
- * @return the result of binding all the variables of these NonTerminals
or an {@code UNSET_LIST} if the term
- * was unset.
+ * @param options the query options containing the values to bind markers
to.
+ * @return the result of binding all the variables of these NonTerminals.
*/
- public List<Terminal> bind(QueryOptions options);
+ Terminals bind(QueryOptions options);
+ /**
+ * A shorter for {@code bind(options).get()}.
+ * We expose it mainly because for constants it can avoid allocating a
temporary
+ * object between the bind and the get.
+ * @param options the query options containing the values to bind markers
to.
+ */
+ List<ByteBuffer> bindAndGet(QueryOptions options);
- public List<ByteBuffer> bindAndGet(QueryOptions options);
+ /**
+ * A shorter for {@code bind(options).getElements()}.
+ * We expose it mainly because for constants it can avoid allocating a
temporary
+ * object between the {@code bind} and the {@code getElements}.
+ * @param options the query options containing the values to bind markers
to.
+ */
+ List<List<ByteBuffer>> bindAndGetElements(QueryOptions options);
/**
- * Creates a {@code Terms} for the specified list marker.
+ * Creates a {@code Terms} containing a single {@code Term}.
*
- * @param marker the list marker
- * @param type the element type
- * @return a {@code Terms} for the specified list marker
+ * @param term the {@code Term}
+ * @return a {@code Terms} containing a single {@code Term}.
*/
- public static Terms ofListMarker(final Lists.Marker marker, final
AbstractType<?> type)
+ static Terms of(final Term term)
{
- return new Terms()
- {
- @Override
- public void addFunctionsTo(List<Function> functions)
- {
- }
+ if (term.isTerminal())
+ return Terminals.of(term == Constants.NULL_VALUE ? null :
(Terminal) term);
+
+ return NonTerminals.of((NonTerminal) term);
+ }
- @Override
- public void collectMarkerSpecification(VariableSpecifications
boundNames)
+ /**
+ * Creates a {@code Terms} containing a set of {@code Term}.
+ *
+ * @param terms the terms
+ * @return a {@code Terms} containing a set of {@code Term}.
+ */
+ static Terms of(final List<Term> terms)
+ {
+ boolean allTerminals = terms.stream().allMatch(Term::isTerminal);
+
+ if (allTerminals)
+ {
+ int size = terms.size();
+ List<Terminal> terminals = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
{
- marker.collectMarkerSpecification(boundNames);
+ Terminal terminal = (Terminal) terms.get(i);
+ terminals.add(terminal == Constants.NULL_VALUE ? null :
terminal);
}
+ return Terminals.of(terminals);
+ }
- @Override
- public List<ByteBuffer> bindAndGet(QueryOptions options)
- {
- Terminal terminal = marker.bind(options);
+ return NonTerminals.of(terms);
+ }
- if (terminal == null)
- return null;
+ /**
+ * Checks if these {@code terms} knows that it contains a single {@code
term}.
+ * <p>
+ * If the instance is a marker it will not know how many terms it
represents and will return false.
+ * @return {@code true} if this {@code terms} know contains a single
{@code term}, {@code false} otherwise.
+ */
+ boolean containsSingleTerm();
- if (terminal == Constants.UNSET_VALUE)
- return UNSET_LIST;
+ /**
+ * If this {@code terms} contains a single term it will be returned
otherwise an UnsupportedOperationException will be thrown.
+ * @return a single term representing the single element of this {@code
terms}.
+ * @throws UnsupportedOperationException if this term does not know how
many terms it contains or contains more than one term
+ */
+ Term asSingleTerm();
- return ((MultiItemTerminal) terminal).getElements();
- }
+ /**
+ * Adds all functions (native and user-defined) of the specified terms to
the list.
+ * @param functions the list to add to
+ */
+ static void addFunctions(Iterable<? extends Term> terms, List<Function>
functions)
+ {
+ for (Term term : terms)
+ {
+ if (term != null)
+ term.addFunctionsTo(functions);
+ }
+ }
- @Override
- public List<Terminal> bind(QueryOptions options)
- {
- Terminal terminal = marker.bind(options);
+ /**
+ * A parsed, non prepared (thus untyped) set of terms.
+ */
+ abstract class Raw implements AssignmentTestable
+ {
+ /**
+ * This method validates this {@code Terms.Raw} is valid for the
provided column
+ * specification and "prepare" this {@code Terms.Raw}, returning the
resulting {@link Terms}.
+ *
+ * @param receiver the "column" the set of terms are supposed to be a
value of. Note
+ * that the ColumnSpecification may not correspond to a real column.
+ * @return the prepared terms
+ */
+ public abstract Terms prepare(String keyspace, ColumnSpecification
receiver) throws InvalidRequestException;
- if (terminal == null)
- return null;
+ /**
+ * @return a String representation of the raw terms that can be used
when reconstructing a CQL query string.
+ */
+ public abstract String getText();
- if (terminal == Constants.UNSET_VALUE)
- return UNSET_LIST;
+ /**
+ * The type of the {@code Terms} if it can be infered.
+ *
+ * @param keyspace the keyspace on which the statement containing
these terms is on.
+ * @return the type of this {@code Terms} if inferrable, or {@code
null}
+ * otherwise (for instance, the type isn't inferrable for a bind
marker. Even for
+ * literals, the exact type is not inferrable since they are valid for
many
+ * different types and so this will return {@code null} too).
+ */
+ public abstract AbstractType<?> getExactTypeIfKnown(String keyspace);
- java.util.function.Function<ByteBuffer, Term.Terminal>
deserializer = deserializer(options.getProtocolVersion());
+ @Override
+ public AbstractType<?> getCompatibleTypeIfKnown(String keyspace)
+ {
+ return getExactTypeIfKnown(keyspace);
+ }
- List<ByteBuffer> boundValues = ((MultiItemTerminal)
terminal).getElements();
- List<Term.Terminal> values = new
ArrayList<>(boundValues.size());
- for (int i = 0, m = boundValues.size(); i < m; i++)
- {
- ByteBuffer buffer = boundValues.get(i);
- Term.Terminal value = buffer == null ? null :
deserializer.apply(buffer);
- values.add(value);
- }
- return values;
- }
+ @Override
+ public String toString()
+ {
+ return getText();
+ }
- public java.util.function.Function<ByteBuffer, Term.Terminal>
deserializer(ProtocolVersion version)
+ public static Raw of(List<? extends Term.Raw> raws)
+ {
+ return new Raw()
{
- if (type.isCollection())
+ @Override
+ public Terms prepare(String keyspace, ColumnSpecification
receiver) throws InvalidRequestException
{
- switch (((CollectionType<?>) type).kind)
+ List<Term> terms = new ArrayList<>(raws.size());
+ for (Term.Raw raw : raws)
{
- case LIST:
- return e -> Lists.Value.fromSerialized(e,
(ListType<?>) type);
- case SET:
- return e -> Sets.Value.fromSerialized(e,
(SetType<?>) type);
- case MAP:
- return e -> Maps.Value.fromSerialized(e,
(MapType<?, ?>) type);
+ terms.add(raw.prepare(keyspace, receiver));
}
- throw new AssertionError();
+ return Terms.of(terms);
}
- return e -> new Constants.Value(e);
- }
- };
+
+ @Override
+ public String getText()
+ {
+ return raws.stream().map(Term.Raw::getText)
+ .collect(Collectors.joining(", ", "(",
")"));
+ }
+
+ @Override
+ public AbstractType<?> getExactTypeIfKnown(String keyspace)
+ {
+ return null;
+ }
+
+ @Override
+ public TestResult testAssignment(String keyspace,
ColumnSpecification receiver)
+ {
+ return AssignmentTestable.TestResult.WEAKLY_ASSIGNABLE;
+ }
+ };
+ }
}
/**
- * Creates a {@code Terms} containing a single {@code Term}.
- *
- * @param term the {@code Term}
- * @return a {@code Terms} containing a single {@code Term}.
+ * Set of terms that contains only terminal terms.
*/
- public static Terms of(final Term term)
+ abstract class Terminals implements Terms
{
- return new Terms()
+ @Override
+ public void collectMarkerSpecification(VariableSpecifications
boundNames) {}
+
+ @Override
+ public final Terminals bind(QueryOptions options)
+ {
+ return this;
+ }
+
+ @Override
+ public List<ByteBuffer> bindAndGet(QueryOptions options)
+ {
+ return get();
+ }
+
+ @Override
+ public List<List<ByteBuffer>> bindAndGetElements(QueryOptions options)
+ {
+ return getElements();
+ }
+
+ /**
+ * @return the serialized values of this {@code Terminals}.
+ */
+ public abstract List<ByteBuffer> get();
+
+ /**
+ * Returns the serialized values of each Term elements, if these term
represents a Collection, Tuple or UDT.
+ * If the terms do not represent multi-elements type the method will
return a list containing the serialized value of the terminals
+ * @return a list containing serialized values of each Term elements
+ */
+ public abstract List<List<ByteBuffer>> getElements();
+
+ /**
+ * Converts these {@code Terminals} into a {@code List} of {@code
Term.Terminal}.
+ * @return a {@code List} of {@code Term.Terminal}.
+ */
+ public abstract List<Term.Terminal> asList();
+
+ /**
+ * Converts a {@code Terminal} into a {@code Terminals}.
+ * @param terminal the {@code Terminal} to convert
+ * @return a {@code Terminals}.
+ */
+ public static Terminals of(Terminal terminal)
+ {
+ return new Terminals()
+ {
+ @Override
+ public List<ByteBuffer> get()
{
- @Override
- public void addFunctionsTo(List<Function> functions)
- {
- term.addFunctionsTo(functions);
- }
+ return Collections.singletonList(terminal == null ? null :
terminal.get());
+ }
- @Override
- public void
collectMarkerSpecification(VariableSpecifications boundNames)
- {
- term.collectMarkerSpecification(boundNames);
- }
+ @Override
+ public List<List<ByteBuffer>> getElements()
+ {
+ return Collections.singletonList(terminal == null ? null :
terminal.getElements());
+ }
+
+ @Override
+ public List<Terminal> asList()
+ {
+ return Collections.singletonList(terminal);
+ }
- @Override
- public List<ByteBuffer> bindAndGet(QueryOptions options)
+ @Override
+ public void addFunctionsTo(List<Function> functions)
+ {
+ if (terminal != null)
+ terminal.addFunctionsTo(functions);
+ }
+
+ @Override
+ public boolean containsSingleTerm()
+ {
+ return true;
+ }
+
+ @Override
+ public Term asSingleTerm()
+ {
+ return terminal;
+ }
+ };
+ }
+
+ public static Terminals of(List<Terminal> terminals)
+ {
+ return new Terminals()
+ {
+ @Override
+ public List<Terminal> asList()
+ {
+ return terminals;
+ }
+
+ @Override
+ public List<ByteBuffer> get()
+ {
+ int size = terminals.size();
+ List<ByteBuffer> buffers = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
{
- return
Collections.singletonList(term.bindAndGet(options));
+ Terminal terminal = terminals.get(i);
+ buffers.add(terminal == null ? null : terminal.get());
}
+ return buffers;
+ }
- @Override
- public List<Terminal> bind(QueryOptions options)
+ @Override
+ public List<List<ByteBuffer>> getElements()
+ {
+ int size = terminals.size();
+ List<List<ByteBuffer>> buffers = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
{
- return Collections.singletonList(term.bind(options));
+ Terminal terminal = terminals.get(i);
+ buffers.add(terminal == null ? null :
terminal.getElements());
}
- };
+ return buffers;
+ }
+
+ @Override
+ public void addFunctionsTo(List<Function> functions)
+ {
+ addFunctions(terminals, functions);
+ }
+
+ @Override
+ public boolean containsSingleTerm()
+ {
+ return terminals.size() == 1;
+ }
+
+ @Override
+ public Terminal asSingleTerm()
+ {
+ if (!containsSingleTerm())
+ throw new UnsupportedOperationException("This terms
contains cannot be converted in a single term");
+ return terminals.get(0);
+ }
+ };
+ }
}
/**
- * Creates a {@code Terms} containing a set of {@code Term}.
- *
- * @param term the {@code Term}
- * @return a {@code Terms} containing a set of {@code Term}.
+ * Set of terms that contains at least one non-terminal term.
*/
- public static Terms of(final List<Term> terms)
+ abstract class NonTerminals implements Terms
{
- return new Terms()
+ public static NonTerminals of(NonTerminal term)
+ {
+ return new NonTerminals()
+ {
+ @Override
+ public void addFunctionsTo(List<Function> functions)
+ {
+ term.addFunctionsTo(functions);
+ }
+
+ @Override
+ public void collectMarkerSpecification(VariableSpecifications
boundNames)
+ {
+ term.collectMarkerSpecification(boundNames);
+ }
+
+ @Override
+ public Terminals bind(QueryOptions options)
+ {
+ return Terminals.of(term.bind(options));
+ }
+
+ @Override
+ public List<ByteBuffer> bindAndGet(QueryOptions options)
+ {
+ return Collections.singletonList(term.bindAndGet(options));
+ }
+
+ @Override
+ public List<List<ByteBuffer>> bindAndGetElements(QueryOptions
options)
+ {
+ return
Collections.singletonList(term.bindAndGetElements(options));
+ }
+
+ @Override
+ public boolean containsSingleTerm()
+ {
+ return true;
+ }
+
+ @Override
+ public Term.NonTerminal asSingleTerm()
+ {
+ return term;
+ }
+ };
+ }
+
+ public static NonTerminals of(List<Term> terms)
+ {
+ return new NonTerminals()
+ {
+ @Override
+ public void addFunctionsTo(List<Function> functions)
+ {
+ addFunctions(terms, functions);
+ }
+
+ @Override
+ public void collectMarkerSpecification(VariableSpecifications
boundNames)
{
- @Override
- public void addFunctionsTo(List<Function> functions)
+ for (int i = 0, m = terms.size(); i < m; i++)
{
- addFunctions(terms, functions);
+ Term term = terms.get(i);
+ term.collectMarkerSpecification(boundNames);
}
+ }
- @Override
- public void
collectMarkerSpecification(VariableSpecifications boundNames)
+ @Override
+ public Terminals bind(QueryOptions options)
+ {
+ int size = terms.size();
+ List<Terminal> terminals = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
{
- for (int i = 0, m = terms.size(); i <m; i++)
- {
- Term term = terms.get(i);
- term.collectMarkerSpecification(boundNames);
- }
+ Term term = terms.get(i);
+ terminals.add(term.bind(options));
}
+ return Terminals.of(terminals);
+ }
- @Override
- public List<Terminal> bind(QueryOptions options)
+ @Override
+ public List<ByteBuffer> bindAndGet(QueryOptions options)
+ {
+ int size = terms.size();
+ List<ByteBuffer> buffers = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
{
- int size = terms.size();
- List<Terminal> terminals = new ArrayList<>(size);
- for (int i = 0; i < size; i++)
- {
- Term term = terms.get(i);
- terminals.add(term.bind(options));
- }
- return terminals;
+ Term term = terms.get(i);
+ buffers.add(term.bindAndGet(options));
}
+ return buffers;
+ }
- @Override
- public List<ByteBuffer> bindAndGet(QueryOptions options)
+ @Override
+ public List<List<ByteBuffer>> bindAndGetElements(QueryOptions
options)
+ {
+ int size = terms.size();
+ List<List<ByteBuffer>> buffers = new ArrayList<>(size);
+ for (int i = 0; i < size; i++)
{
- int size = terms.size();
- List<ByteBuffer> buffers = new ArrayList<>(size);
- for (int i = 0; i < size; i++)
- {
- Term term = terms.get(i);
- buffers.add(term.bindAndGet(options));
- }
- return buffers;
+ Term term = terms.get(i);
+ buffers.add(term.bindAndGetElements(options));
}
- };
- }
+ return buffers;
+ }
- /**
- * Adds all functions (native and user-defined) of the specified terms to
the list.
- * @param functions the list to add to
- */
- public static void addFunctions(Iterable<Term> terms, List<Function>
functions)
- {
- for (Term term : terms)
- {
- if (term != null)
- term.addFunctionsTo(functions);
- }
- }
+ @Override
+ public boolean containsSingleTerm()
+ {
+ return terms.size() == 1;
+ }
- public static ByteBuffer asBytes(String keyspace, String term,
AbstractType type)
- {
- ColumnSpecification receiver = new ColumnSpecification(keyspace,
SchemaConstants.DUMMY_KEYSPACE_OR_TABLE_NAME, new ColumnIdentifier("(dummy)",
true), type);
- Term.Raw rawTerm = CQLFragmentParser.parseAny(CqlParser::term, term,
"CQL term");
- return rawTerm.prepare(keyspace,
receiver).bindAndGet(QueryOptions.DEFAULT);
+ @Override
+ public Term asSingleTerm()
+ {
+ if (!containsSingleTerm())
+ throw new UnsupportedOperationException("This terms
contains cannot be converted in a single term");
Review Comment:
terms -> term?
--
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]
