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