adelapena commented on code in PR #2655:
URL: https://github.com/apache/cassandra/pull/2655#discussion_r1313239973


##########
src/java/org/apache/cassandra/cql3/Terms.java:
##########
@@ -47,225 +47,510 @@ public Object get(int index)
         @Override
         public int size()
         {
-            return 0;
+            throw new UnsupportedOperationException();
+        }
+    };
+
+    Terminals UNSET_TERMINALS = new Terminals()
+    {
+        @Override
+        public List<ByteBuffer> get()
+        {
+            return (List<ByteBuffer>) UNSET_LIST;
+        }
+
+        @Override
+        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.
+     */
+    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);
+
+    /**
+     * 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.
      */
-    public List<Terminal> bind(QueryOptions options);
+    List<List<ByteBuffer>> bindAndGetElements(QueryOptions options);
 
+    /**
+     * Whether those terms are terminal (this is a shortcut for {@code this 
instanceof Terms.Terminals}).
+     */
+    default boolean areTerminals()
+    {
+        return false; // overriden below by Terminals
+    }
+
+    /**
+     * Creates a {@code Terms} containing a single {@code Term}.
+     *
+     * @param term the {@code Term}
+     * @return a {@code Terms} containing a single {@code Term}.
+     */
+    static Terms of(final Term term)
+    {
+        if (term.isTerminal())
+            return Terminals.of(term == Constants.NULL_VALUE ? null : 
(Terminal) term);
 
-    public List<ByteBuffer> bindAndGet(QueryOptions options);
+        return NonTerminals.of((NonTerminal) term);
+    }
 
     /**
-     * Creates a {@code Terms} for the specified list marker.
+     * Creates a {@code Terms} containing a set of {@code Term}.
      *
-     * @param marker the list  marker
-     * @param type the element type
-     * @return a {@code Terms} for the specified list marker
+     * @param terms the terms
+     * @return a {@code Terms} containing a set of {@code Term}.
      */
-    public static Terms ofListMarker(final Lists.Marker marker, final 
AbstractType<?> type)
+    static Terms of(final List<Term> terms)
     {
-        return new Terms()
+        boolean allTerminals = terms.stream().allMatch(Term::isTerminal);
+
+        if (allTerminals)
         {
-            @Override
-            public void addFunctionsTo(List<Function> functions)
+            int size = terms.size();
+            List<Terminal> terminals = new ArrayList<>(size);
+            for (int i = 0; i < size; i++)
             {
+                Terminal terminal = (Terminal) terms.get(i);
+                terminals.add(terminal == Constants.NULL_VALUE ? null : 
terminal);
             }
+            return Terminals.of(terminals);
+        }
 
-            @Override
-            public void collectMarkerSpecification(VariableSpecifications 
boundNames)
-            {
-                marker.collectMarkerSpecification(boundNames);
-            }
+        return NonTerminals.of(terms);
+    }
 
-            @Override
-            public List<ByteBuffer> bindAndGet(QueryOptions options)
+    /**
+     * Checks if this {@code terms} know 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 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();
+
+    /**
+     * 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);
+        }
+    }
+
+    /**
+     * 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
+         *
+         * @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;
+
+        /**
+         * @return a String representation of the raw terms that can be used 
when reconstructing a CQL query string.
+         */
+        public abstract String getText();
+
+        /**
+         * 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);
+
+        @Override
+        public AbstractType<?> getCompatibleTypeIfKnown(String keyspace)
+        {
+            return getExactTypeIfKnown(keyspace);
+        }
+
+        @Override
+        public String toString()
+        {
+            return getText();
+        }
+
+        public static Raw of(List<? extends Term.Raw> raws)
+        {
+            return new Raw()
             {
-                Terminal terminal = marker.bind(options);
+                @Override
+                public Terms prepare(String keyspace, ColumnSpecification 
receiver) throws InvalidRequestException
+                {
+                    List<Term> terms = new ArrayList<>(raws.size());
+                    for (Term.Raw raw : raws)
+                    {
+                        terms.add(raw.prepare(keyspace, receiver));
+                    }
+                    return Terms.of(terms);
+                }
+
+                @Override
+                public String getText()
+                {
+                    return raws.stream().map(Term.Raw::getText)
+                                        .collect(Collectors.joining(", ", "(", 
")"));
+                }
 
-                if (terminal == null)
+                @Override
+                public AbstractType<?> getExactTypeIfKnown(String keyspace)
+                {
                     return null;
+                }
+
+                @Override
+                public TestResult testAssignment(String keyspace, 
ColumnSpecification receiver)
+                {
+                    return AssignmentTestable.TestResult.WEAKLY_ASSIGNABLE;
+                }
+            };
+        }
+    }
+
+    abstract class Terminals implements Terms
+    {
+        public final boolean areTerminals()
+        {
+            return true;
+        }
+        @Override
+        public void collectMarkerSpecification(VariableSpecifications 
boundNames) {}
 
-                if (terminal == Constants.UNSET_VALUE)
-                    return UNSET_LIST;
+        @Override
+        public final Terminals bind(QueryOptions options)
+        {
+            return this;
+        }
 
-                return ((MultiItemTerminal) terminal).getElements();
-            }
+        @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();
 
-            @Override
-            public List<Terminal> bind(QueryOptions options)
+        public abstract List<Term.Terminal> asList();
+
+        public static Terminals of(Terminal terminal)
+        {
+            return new Terminals()
             {
-                Terminal terminal = marker.bind(options);
+                @Override
+                public List<ByteBuffer> get()
+                {
+                    return Collections.singletonList(terminal == null ? null : 
terminal.get());
+                }
 
-                if (terminal == null)
-                    return null;
+                @Override
+                public List<List<ByteBuffer>> getElements()
+                {
+                    return Collections.singletonList(terminal == null ? null : 
terminal.getElements());
+                }
 
-                if (terminal == Constants.UNSET_VALUE)
-                    return UNSET_LIST;
+                @Override
+                public List<Terminal> asList()
+                {
+                    return Collections.singletonList(terminal);
+                }
 
-                java.util.function.Function<ByteBuffer, Term.Terminal> 
deserializer = deserializer(options.getProtocolVersion());
+                @Override
+                public void addFunctionsTo(List<Function> functions)
+                {
+                    if (terminal != null)
+                        terminal.addFunctionsTo(functions);
+                }
 
-                List<ByteBuffer> boundValues = ((MultiItemTerminal) 
terminal).getElements();
-                List<Term.Terminal> values = new 
ArrayList<>(boundValues.size());
-                for (int i = 0, m = boundValues.size(); i < m; i++)
+                @Override
+                public boolean containsSingleTerm()
                 {
-                    ByteBuffer buffer = boundValues.get(i);
-                    Term.Terminal value = buffer == null ? null : 
deserializer.apply(buffer);
-                    values.add(value);
+                    return true;
                 }
-                return values;
-            }
 
-            public java.util.function.Function<ByteBuffer, Term.Terminal> 
deserializer(ProtocolVersion version)
+                @Override
+                public Term asSingleTerm()
+                {
+                    return terminal;
+                }
+            };
+        }
+
+        public static Terminals of(List<Terminal> terminals)
+        {
+            return new Terminals()
             {
-                if (type.isCollection())
+                @Override
+                public List<Terminal> asList()
                 {
-                    switch (((CollectionType<?>) type).kind)
-                    {
-                        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);
-                    }
-                    throw new AssertionError();
+                    return terminals;
                 }
-                return e -> new Constants.Value(e);
-            }
-        };
-    }
 
-    /**
-     * Creates a {@code Terms} containing a single {@code Term}.
-     *
-     * @param term the {@code Term}
-     * @return a {@code Terms} containing a single {@code Term}.
-     */
-    public static Terms of(final Term term)
-    {
-        return new Terms()
+                @Override
+                public List<ByteBuffer> get()
                 {
-                    @Override
-                    public void addFunctionsTo(List<Function> functions)
+                    int size = terminals.size();
+                    List<ByteBuffer> buffers = new ArrayList<>(size);
+                    for (int i = 0; i < size; i++)
                     {
-                        term.addFunctionsTo(functions);
+                        Terminal terminal = terminals.get(i);
+                        buffers.add(terminal == null ? null : terminal.get());
                     }
+                    return buffers;
+                }
 
-                    @Override
-                    public void 
collectMarkerSpecification(VariableSpecifications boundNames)
+                @Override
+                public List<List<ByteBuffer>> getElements()
+                {
+                    int size = terminals.size();
+                    List<List<ByteBuffer>> buffers = new ArrayList<>(size);
+                    for (int i = 0; i < size; i++)
                     {
-                        term.collectMarkerSpecification(boundNames);
+                        Terminal terminal = terminals.get(i);
+                        buffers.add(terminal == null ? null : 
terminal.getElements());
                     }
+                    return buffers;
+                }
 
-                    @Override
-                    public List<ByteBuffer> bindAndGet(QueryOptions options)
-                    {
-                        return 
Collections.singletonList(term.bindAndGet(options));
-                    }
+                @Override
+                public void addFunctionsTo(List<Function> functions)
+                {
+                    addFunctions(terminals, functions);
+                }
 
-                    @Override
-                    public List<Terminal> bind(QueryOptions options)
-                    {
-                        return Collections.singletonList(term.bind(options));
-                    }
-                };
+                @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}.
-     */
-    public static Terms of(final List<Term> terms)
+    abstract class NonTerminals implements Terms

Review Comment:
   It might be useful to clarify in the class JavaDoc whether it's expected 
that all the terms in this class are expected to be non-terminal, or if it 
might contain at least one non-terminal.



-- 
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]

Reply via email to