bbotella commented on code in PR #4115:
URL: https://github.com/apache/cassandra/pull/4115#discussion_r2071677801


##########
src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java:
##########
@@ -255,6 +279,10 @@ public ColumnConstraints deserialize(DataInputPlus in, 
Version version) throws I
                                                                        
.deserialize(in, version);
                 columnConstraints.add(constraint);
             }
+
+            // we are not setting column name here on purpose

Review Comment:
   Neat trick



##########
src/antlr/Parser.g:
##########
@@ -997,6 +1018,12 @@ columnMaskArguments[List<Term.Raw> arguments]
     : '('  ')' | '(' c=term { arguments.add(c); } (',' cn=term { 
arguments.add(cn); })* ')'
     ;
 
+columnConstraintsArguments[List<String> arguments]
+    : '('  ')'
+    | '(' c=term { try { arguments.add(c.toString()); } catch (Throwable t) { 
throw new SyntaxException("Constraint function parameters need to be 
strings."); }; } (',' cn=term { try { arguments.add(cn.toString()); } catch 
(Throwable t) { throw new SyntaxException("Constraint function parameters need 
to be strings."); }; })* ')'
+    | '(' ci=ident { throw new SyntaxException("Constraint function parameters 
need to be strings."); } (',' cni=ident)* ')'

Review Comment:
   Do we need to check for the `(',' cni=ident)*` to be strings as well?



##########
src/java/org/apache/cassandra/cql3/constraints/ConstraintFunction.java:
##########
@@ -100,4 +113,44 @@ public void validate(ColumnMetadata columnMetadata, String 
term) throws InvalidC
      * @return supported types for given constraint
      */
     public abstract List<AbstractType<?>> getSupportedTypes();
+
+    /**
+     * Tells whether implementation supports specifying arguments on its 
function.
+     * <br>
+     * In this case, this function will return "true"
+     * <pre>
+     *     val int check length() < 1024
+     * </pre>
+     *
+     * In this case, this function will return "false"
+     * <pre>
+     *     val int check someconstraint('abc', 'def')
+     * </pre>
+     * @return true if this constraint does not accept any parameters, false 
otherwise.
+     */
+    public boolean isParameterless() { return true; }

Review Comment:
   Do you think it would be worth having ParameterlessConstraintFunction class 
and NonParameterlessConstraintFunction inheriting from ConstraintFunction that 
basically give this method already prefilled for you?



-- 
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: pr-unsubscr...@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org

Reply via email to