adelapena commented on code in PR #2655: URL: https://github.com/apache/cassandra/pull/2655#discussion_r1328529687
########## src/java/org/apache/cassandra/cql3/Marker.java: ########## @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.cassandra.cql3; + +import java.nio.ByteBuffer; +import java.util.List; + +import org.apache.cassandra.cql3.functions.Function; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.MultiElementType; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.serializers.MarshalException; +import org.apache.cassandra.utils.ByteBufferUtil; + +import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse; + +/** + * A placeholder, also called bind marker, for a single value represented in CQL as '?' for an unnamed marker or ':<name>' for a named marker. + * For example, 'SELECT ... WHERE pk = ?' or 'SELECT ... WHERE pk = :myKey '. + */ +public final class Marker extends Term.NonTerminal +{ + private final int bindIndex; Review Comment: Can we add JavaDoc for this attribute? ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); /** - * A shorter for bind(values).get(). - * We expose it mainly because for constants it can avoids allocating a temporary + * A shorter for bind(options).get(). + * We expose it mainly because for constants it can avoid allocating a temporary * object between the bind and the get (note that we still want to be able * to separate bind and get for collections). */ - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException; + ByteBuffer bindAndGet(QueryOptions options); /** - * Whether or not that term contains at least one bind marker. - * + * A shorter for bind(options).getElements(). + * We expose it mainly because for constants it can avoid allocating a temporary + * object between the bind and the getElements . Review Comment: ```suggestion * object between the bind and the getElements. ``` ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); /** - * A shorter for bind(values).get(). - * We expose it mainly because for constants it can avoids allocating a temporary + * A shorter for bind(options).get(). Review Comment: ```suggestion * A shorter for {@code bind(options).get()}. * <p> ``` ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -188,35 +208,55 @@ public boolean isTerminal() /** * @return the serialized value of this terminal. */ - public abstract ByteBuffer get(ProtocolVersion version) throws InvalidRequestException; + public abstract ByteBuffer get(); - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException + /** Review Comment: Nit: all JavaDoc on this file seems formatted to width 80, except for this one. By the way, the comments across the codebase sometimes use width 80, sometimes 120. I don't know if we have an agreed standard. ########## src/java/org/apache/cassandra/cql3/InMarker.java: ########## @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.cql3; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +import org.apache.cassandra.cql3.functions.Function; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.ByteBufferAccessor; +import org.apache.cassandra.db.marshal.ListType; +import org.apache.cassandra.db.marshal.MultiElementType; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.utils.ByteBufferUtil; + +/** + * A CQL named or unnamed bind marker for an {@code IN} restriction. + * For example, 'SELECT ... WHERE pk IN ?' or 'SELECT ... WHERE pk IN :myKey '. + */ +public final class InMarker extends Terms.NonTerminals +{ + private final int bindIndex; + private final ColumnSpecification receiver; + + private InMarker(int bindIndex, ColumnSpecification receiver) + { + this.bindIndex = bindIndex; + this.receiver = receiver; + } + + @Override + public void addFunctionsTo(List<Function> functions) {} + + @Override + public void collectMarkerSpecification(VariableSpecifications boundNames) + { + boundNames.add(bindIndex, receiver); + } + + @Override + public Terminals bind(QueryOptions options) + { + ByteBuffer values = options.getValues().get(bindIndex); + if (values == null) + return null; + + if (values == ByteBufferUtil.UNSET_BYTE_BUFFER) + return UNSET_TERMINALS; + + ListType<?> type = (ListType) receiver.type; Review Comment: ```suggestion ListType<?> type = (ListType<?>) receiver.type; ``` ########## src/java/org/apache/cassandra/cql3/InMarker.java: ########## @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.cql3; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +import org.apache.cassandra.cql3.functions.Function; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.ByteBufferAccessor; +import org.apache.cassandra.db.marshal.ListType; +import org.apache.cassandra.db.marshal.MultiElementType; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.utils.ByteBufferUtil; + +/** + * A CQL named or unnamed bind marker for an {@code IN} restriction. + * For example, 'SELECT ... WHERE pk IN ?' or 'SELECT ... WHERE pk IN :myKey '. + */ +public final class InMarker extends Terms.NonTerminals +{ + private final int bindIndex; + private final ColumnSpecification receiver; + + private InMarker(int bindIndex, ColumnSpecification receiver) + { + this.bindIndex = bindIndex; + this.receiver = receiver; + } + + @Override + public void addFunctionsTo(List<Function> functions) {} + + @Override + public void collectMarkerSpecification(VariableSpecifications boundNames) + { + boundNames.add(bindIndex, receiver); + } + + @Override + public Terminals bind(QueryOptions options) + { + ByteBuffer values = options.getValues().get(bindIndex); + if (values == null) + return null; + + if (values == ByteBufferUtil.UNSET_BYTE_BUFFER) + return UNSET_TERMINALS; + + ListType<?> type = (ListType) receiver.type; + return toTerminals(values, type, terminalConverter(type.getElementsType())); + } + + private <T> Terminals toTerminals(ByteBuffer value, + ListType<T> type, + java.util.function.Function<ByteBuffer, Term.Terminal> terminalConverter) + { + List<T> elements = type.getSerializer().deserialize(value, ByteBufferAccessor.instance); + List<Term.Terminal> terminals = new ArrayList<>(elements.size()); + for (T element : elements) + { + terminals.add(element == null ? null : terminalConverter.apply(type.getElementsType().decompose(element))); + } + return Terminals.of(terminals); + } + + public static java.util.function.Function<ByteBuffer, Term.Terminal> terminalConverter(AbstractType<?> type) + { + if (type instanceof MultiElementType<?>) + return e -> MultiElements.Value.fromSerialized(e, (MultiElementType<?>) type); + + return Constants.Value::new; + } + + @Override + public List<ByteBuffer> bindAndGet(QueryOptions options) + { + Terminals terminals = bind(options); + return terminals == null ? null : terminals.get(); + } + + @Override + public List<List<ByteBuffer>> bindAndGetElements(QueryOptions options) + { + Terminals terminals = bind(options); + return terminals == null ? null : terminals.getElements(); + } + + @Override + public boolean containsSingleTerm() + { + return false; + } + + @Override + public Term asSingleTerm() + { + throw new UnsupportedOperationException(); + } + + /** + * A raw placeholder for multiple values of the same type for a single column. + * For example, 'SELECT ... WHERE user_id IN ?'. Review Comment: ```suggestion * For example, {@code SELECT ... WHERE user_id IN ?'}. ``` ########## src/java/org/apache/cassandra/cql3/Relation.java: ########## @@ -211,41 +208,41 @@ protected abstract Restriction newSliceRestriction(TableMetadata table, /** * Converts the specified <code>Raw</code> into a <code>Term</code>. - * @param receivers the columns to which the values must be associated at + * @param receiver the columns to which the values must be associated at Review Comment: ```suggestion * @param receiver the column to which the values must be associated at ``` ########## src/java/org/apache/cassandra/cql3/InMarker.java: ########## @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.cql3; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +import org.apache.cassandra.cql3.functions.Function; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.ByteBufferAccessor; +import org.apache.cassandra.db.marshal.ListType; +import org.apache.cassandra.db.marshal.MultiElementType; +import org.apache.cassandra.exceptions.InvalidRequestException; +import org.apache.cassandra.utils.ByteBufferUtil; + +/** + * A CQL named or unnamed bind marker for an {@code IN} restriction. + * For example, 'SELECT ... WHERE pk IN ?' or 'SELECT ... WHERE pk IN :myKey '. + */ +public final class InMarker extends Terms.NonTerminals +{ + private final int bindIndex; + private final ColumnSpecification receiver; + + private InMarker(int bindIndex, ColumnSpecification receiver) + { + this.bindIndex = bindIndex; + this.receiver = receiver; + } + + @Override + public void addFunctionsTo(List<Function> functions) {} + + @Override + public void collectMarkerSpecification(VariableSpecifications boundNames) + { + boundNames.add(bindIndex, receiver); + } + + @Override + public Terminals bind(QueryOptions options) + { + ByteBuffer values = options.getValues().get(bindIndex); + if (values == null) + return null; + + if (values == ByteBufferUtil.UNSET_BYTE_BUFFER) + return UNSET_TERMINALS; + + ListType<?> type = (ListType) receiver.type; + return toTerminals(values, type, terminalConverter(type.getElementsType())); + } + + private <T> Terminals toTerminals(ByteBuffer value, + ListType<T> type, + java.util.function.Function<ByteBuffer, Term.Terminal> terminalConverter) + { + List<T> elements = type.getSerializer().deserialize(value, ByteBufferAccessor.instance); + List<Term.Terminal> terminals = new ArrayList<>(elements.size()); + for (T element : elements) + { + terminals.add(element == null ? null : terminalConverter.apply(type.getElementsType().decompose(element))); + } + return Terminals.of(terminals); + } + + public static java.util.function.Function<ByteBuffer, Term.Terminal> terminalConverter(AbstractType<?> type) + { + if (type instanceof MultiElementType<?>) Review Comment: We could have an `AbstractType#isMultiElement` method, analogous to `AbstractType#isMultiCell`. ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); Review Comment: The `@return` part of this JavaDoc seems to need an update. ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -188,35 +208,55 @@ public boolean isTerminal() /** * @return the serialized value of this terminal. */ - public abstract ByteBuffer get(ProtocolVersion version) throws InvalidRequestException; + public abstract ByteBuffer get(); - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException + /** + * Returns the serialized values of this Term elements, if this term represents a Collection, Tuple or UDT. + * If this term does not represent a multi-elements type it will return a list containing the serialized value of this terminal + * @return a list containing serialized values of this Term elements + */ + public List<ByteBuffer> getElements() { - return get(options.getProtocolVersion()); + ByteBuffer value = get(); + return value == null ? Collections.emptyList() : Collections.singletonList(value); } - } - public abstract class MultiItemTerminal extends Terminal - { - public abstract List<ByteBuffer> getElements(); + public ByteBuffer bindAndGet(QueryOptions options) + { + return get(); + } + + public List<ByteBuffer> bindAndGetElements(QueryOptions options) + { + return getElements(); + } } /** - * A non terminal term, i.e. a term that can only be reduce to a byte buffer + * A non-terminal term, i.e. a term that can only be reduced to a byte buffer * at execution time. - * + * <p> * We have the following type of NonTerminal: - * - marker for a constant value - * - marker for a collection value (list, set, map) - * - a function having bind marker - * - a non pure function (even if it doesn't have bind marker - see #5616) + * <ul> + * <li>marker for a constant value</li> + * <li>marker for a collection value (list, set, map)</li> Review Comment: Now multi-element data type value. ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); /** - * A shorter for bind(values).get(). - * We expose it mainly because for constants it can avoids allocating a temporary + * A shorter for bind(options).get(). + * We expose it mainly because for constants it can avoid allocating a temporary * object between the bind and the get (note that we still want to be able * to separate bind and get for collections). */ - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException; + ByteBuffer bindAndGet(QueryOptions options); /** - * Whether or not that term contains at least one bind marker. - * + * A shorter for bind(options).getElements(). + * We expose it mainly because for constants it can avoid allocating a temporary + * object between the bind and the getElements . + */ + List<ByteBuffer> bindAndGetElements(QueryOptions options); + + /** + * Whether that term contains at least one bind marker. + * <p> * Note that this is slightly different from being or not a NonTerminal, - * because calls to non pure functions will be NonTerminal (see #5616) + * because calls to non-pure functions will be NonTerminal (see #5616) Review Comment: ```suggestion * because calls to non-pure functions will be {@link NonTerminal} (see #5616) ``` ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -144,26 +167,23 @@ public boolean equals(Object o) } } - public abstract class MultiColumnRaw extends Term.Raw - { - public abstract Term prepare(String keyspace, List<? extends ColumnSpecification> receiver) throws InvalidRequestException; - } - /** * A terminal term, one that can be reduced to a byte buffer directly. - * + * <p> * This includes most terms that don't have a bind marker (an exception - * being delayed call for non pure function that are NonTerminal even + * being delayed call for non-pure function that are NonTerminal even * if they don't have bind markers). - * + * <p> * This can be only one of: - * - a constant value - * - a collection value - * + * <ul> + * <li>a constant value</li> + * <li>a collection value</li> Review Comment: A multi-element/multi-value type value? ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); /** - * A shorter for bind(values).get(). - * We expose it mainly because for constants it can avoids allocating a temporary + * A shorter for bind(options).get(). + * We expose it mainly because for constants it can avoid allocating a temporary * object between the bind and the get (note that we still want to be able * to separate bind and get for collections). */ - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException; + ByteBuffer bindAndGet(QueryOptions options); /** - * Whether or not that term contains at least one bind marker. - * + * A shorter for bind(options).getElements(). + * We expose it mainly because for constants it can avoid allocating a temporary + * object between the bind and the getElements . + */ + List<ByteBuffer> bindAndGetElements(QueryOptions options); + + /** + * Whether that term contains at least one bind marker. + * <p> * Note that this is slightly different from being or not a NonTerminal, - * because calls to non pure functions will be NonTerminal (see #5616) + * because calls to non-pure functions will be NonTerminal (see #5616) * even if they don't have bind markers. */ - public abstract boolean containsBindMarker(); + boolean containsBindMarker(); /** * Whether that term is terminal (this is a shortcut for {@code this instanceof Term.Terminal}). */ - default public boolean isTerminal() + default boolean isTerminal() { return false; // overriden below by Terminal } - public void addFunctionsTo(List<Function> functions); + /** + * Adds the functions used by this {@code Term} to the list of functions. Review Comment: ```suggestion * Adds the functions used by this {@link Term} to the list of functions. ``` ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); /** - * A shorter for bind(values).get(). - * We expose it mainly because for constants it can avoids allocating a temporary + * A shorter for bind(options).get(). + * We expose it mainly because for constants it can avoid allocating a temporary * object between the bind and the get (note that we still want to be able * to separate bind and get for collections). */ - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException; + ByteBuffer bindAndGet(QueryOptions options); /** - * Whether or not that term contains at least one bind marker. - * + * A shorter for bind(options).getElements(). Review Comment: ```suggestion * A shorter for {@code bind(options).getElements()}. * <p> ``` ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -41,55 +42,77 @@ public interface Term * @param boundNames the variables specification where to collect the * bind variables of this term in. */ - public void collectMarkerSpecification(VariableSpecifications boundNames); + void collectMarkerSpecification(VariableSpecifications boundNames); /** - * Bind the values in this term to the values contained in {@code values}. + * Bind the values in this term to the values contained in the {@code options}. * This is obviously a no-op if the term is Terminal. * * @param options the values to bind markers to. * @return the result of binding all the variables of this NonTerminal (or * 'this' if the term is terminal). */ - public Terminal bind(QueryOptions options) throws InvalidRequestException; + Terminal bind(QueryOptions options); /** - * A shorter for bind(values).get(). - * We expose it mainly because for constants it can avoids allocating a temporary + * A shorter for bind(options).get(). + * We expose it mainly because for constants it can avoid allocating a temporary * object between the bind and the get (note that we still want to be able * to separate bind and get for collections). */ - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException; + ByteBuffer bindAndGet(QueryOptions options); /** - * Whether or not that term contains at least one bind marker. - * + * A shorter for bind(options).getElements(). + * We expose it mainly because for constants it can avoid allocating a temporary + * object between the bind and the getElements . + */ + List<ByteBuffer> bindAndGetElements(QueryOptions options); + + /** + * Whether that term contains at least one bind marker. + * <p> * Note that this is slightly different from being or not a NonTerminal, - * because calls to non pure functions will be NonTerminal (see #5616) + * because calls to non-pure functions will be NonTerminal (see #5616) * even if they don't have bind markers. */ - public abstract boolean containsBindMarker(); + boolean containsBindMarker(); /** * Whether that term is terminal (this is a shortcut for {@code this instanceof Term.Terminal}). */ - default public boolean isTerminal() + default boolean isTerminal() { return false; // overriden below by Terminal } - public void addFunctionsTo(List<Function> functions); + /** + * Adds the functions used by this {@code Term} to the list of functions. + * <p> + * This method is used to discover prepare statements function dependencies on schema updates. + * @param functions the list of functions to add to + */ + void addFunctionsTo(List<Function> functions); + + 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); + } /** * A parsed, non prepared (thus untyped) term. - * + * <p> * This can be one of: - * - a constant - * - a collection literal - * - a function call - * - a marker + * <ul> + * <li>a constant</li> + * <li>a collection literal</li> Review Comment: Now a multi-element type literal :) ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -188,35 +208,55 @@ public boolean isTerminal() /** * @return the serialized value of this terminal. */ - public abstract ByteBuffer get(ProtocolVersion version) throws InvalidRequestException; + public abstract ByteBuffer get(); - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException + /** + * Returns the serialized values of this Term elements, if this term represents a Collection, Tuple or UDT. + * If this term does not represent a multi-elements type it will return a list containing the serialized value of this terminal + * @return a list containing serialized values of this Term elements + */ + public List<ByteBuffer> getElements() { - return get(options.getProtocolVersion()); + ByteBuffer value = get(); + return value == null ? Collections.emptyList() : Collections.singletonList(value); } - } - public abstract class MultiItemTerminal extends Terminal - { - public abstract List<ByteBuffer> getElements(); + public ByteBuffer bindAndGet(QueryOptions options) + { + return get(); + } + + public List<ByteBuffer> bindAndGetElements(QueryOptions options) + { + return getElements(); + } } /** - * A non terminal term, i.e. a term that can only be reduce to a byte buffer + * A non-terminal term, i.e. a term that can only be reduced to a byte buffer * at execution time. - * + * <p> * We have the following type of NonTerminal: - * - marker for a constant value - * - marker for a collection value (list, set, map) - * - a function having bind marker - * - a non pure function (even if it doesn't have bind marker - see #5616) + * <ul> + * <li>marker for a constant value</li> + * <li>marker for a collection value (list, set, map)</li> + * <li>a function having bind marker</li> + * <li>a non-pure function (even if it doesn't have bind marker - see #5616)</li> Review Comment: ```suggestion * <li>a function having bind markers</li> * <li>a non-pure function (even if it doesn't have bind markers - see #5616)</li> ``` ########## src/java/org/apache/cassandra/cql3/MultiColumnRelation.java: ########## @@ -49,22 +47,19 @@ public class MultiColumnRelation extends Relation { private final List<ColumnIdentifier> entities; - /** A Tuples.Literal or Tuples.Raw marker */ - private final Term.MultiColumnRaw valuesOrMarker; + /** A litteral or raw marker */ Review Comment: ```suggestion /** A literal or raw marker */ ``` ########## src/java/org/apache/cassandra/cql3/Term.java: ########## @@ -188,35 +208,55 @@ public boolean isTerminal() /** * @return the serialized value of this terminal. */ - public abstract ByteBuffer get(ProtocolVersion version) throws InvalidRequestException; + public abstract ByteBuffer get(); - public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException + /** + * Returns the serialized values of this Term elements, if this term represents a Collection, Tuple or UDT. + * If this term does not represent a multi-elements type it will return a list containing the serialized value of this terminal + * @return a list containing serialized values of this Term elements + */ + public List<ByteBuffer> getElements() { - return get(options.getProtocolVersion()); + ByteBuffer value = get(); + return value == null ? Collections.emptyList() : Collections.singletonList(value); } - } - public abstract class MultiItemTerminal extends Terminal - { - public abstract List<ByteBuffer> getElements(); + public ByteBuffer bindAndGet(QueryOptions options) + { + return get(); + } + + public List<ByteBuffer> bindAndGetElements(QueryOptions options) + { + return getElements(); + } } /** - * A non terminal term, i.e. a term that can only be reduce to a byte buffer + * A non-terminal term, i.e. a term that can only be reduced to a byte buffer * at execution time. - * + * <p> * We have the following type of NonTerminal: - * - marker for a constant value - * - marker for a collection value (list, set, map) - * - a function having bind marker - * - a non pure function (even if it doesn't have bind marker - see #5616) + * <ul> + * <li>marker for a constant value</li> + * <li>marker for a collection value (list, set, map)</li> + * <li>a function having bind marker</li> + * <li>a non-pure function (even if it doesn't have bind marker - see #5616)</li> + * </ul> */ - public abstract class NonTerminal implements Term + abstract class NonTerminal implements Term { public ByteBuffer bindAndGet(QueryOptions options) throws InvalidRequestException { Terminal t = bind(options); - return t == null ? null : t.get(options.getProtocolVersion()); + return t == null ? null : t.get(); Review Comment: Nit: can we add `@Override` to the method? ########## src/java/org/apache/cassandra/cql3/MultiColumnRelation.java: ########## @@ -74,37 +69,25 @@ private MultiColumnRelation(List<ColumnIdentifier> entities, Operator relationTy * } * @param entities the columns on the LHS of the relation * @param relationType the relation operator - * @param valuesOrMarker a Tuples.Literal instance or a Tuples.Raw marker + * @param value a litteral or raw marker Review Comment: ```suggestion * @param value a literal or raw marker ``` ########## src/java/org/apache/cassandra/cql3/MultiColumnRelation.java: ########## @@ -74,37 +69,25 @@ private MultiColumnRelation(List<ColumnIdentifier> entities, Operator relationTy * } * @param entities the columns on the LHS of the relation * @param relationType the relation operator - * @param valuesOrMarker a Tuples.Literal instance or a Tuples.Raw marker + * @param value a litteral or raw marker * @return a new <code>MultiColumnRelation</code> instance */ - public static MultiColumnRelation createNonInRelation(List<ColumnIdentifier> entities, Operator relationType, Term.MultiColumnRaw valuesOrMarker) + public static MultiColumnRelation createNonInRelation(List<ColumnIdentifier> entities, Operator relationType, Term.Raw value) { assert relationType != Operator.IN; - return new MultiColumnRelation(entities, relationType, valuesOrMarker, null, null); + return new MultiColumnRelation(entities, relationType, value, null); } /** - * Creates a multi-column IN relation with a list of IN values or markers. + * Creates a multi-column IN relation with either a marker for the IN values or a list of IN values or markers . Review Comment: ```suggestion * Creates a multi-column IN relation with either a marker for the IN values or a list of IN values or markers . ``` ```suggestion * Creates a multi-column IN relation with either a marker for the IN values or a list of IN values or markers. ``` ########## src/java/org/apache/cassandra/cql3/Relation.java: ########## @@ -44,7 +41,7 @@ public Operator operator() /** * Returns the list of raw IN values for this relation, or null if this is not an IN relation. */ - public abstract List<? extends Term.Raw> getInValues(); + public abstract Terms.Raw getInValues(); Review Comment: The method `Relation#isMultiColumn` is unused, as before the patch. We can probably remove it. -- 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]

