ekaterinadimitrova2 commented on code in PR #2398:
URL: https://github.com/apache/cassandra/pull/2398#discussion_r1224306151
##########
test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java:
##########
@@ -48,4 +51,89 @@ public void testInvalidBloomFilterFPRatio() throws Throwable
// sanity check
createTable("CREATE TABLE %s (a int PRIMARY KEY, b int) WITH
bloom_filter_fp_chance = 0.1");
}
+
+ @Test
+ public void testCreateTableErrorOnNonClusterKey()
Review Comment:
There is plenty of repetition. We can improve readability by having a
utility method and splitting the test into a few small ones; each test name
will reflect exactly what we test. Tomek already has something similar in our
fork. You can check it and get inspired by it. We can mark him as then
co-author on the ticket.
##########
src/java/org/apache/cassandra/cql3/statements/CreateTableStatement.java:
##########
@@ -345,6 +345,14 @@ public ParsedStatement.Prepared prepare(Types udts) throws
RequestValidationExce
if (properties.definedOrdering.size() > columnAliases.size())
throw new InvalidRequestException("Only clustering key
columns can be defined in CLUSTERING ORDER directive");
+ for (ColumnIdentifier orderId:
properties.definedOrdering.keySet())
+ {
+ if (!columnAliases.contains(orderId))
+ {
+ throw new InvalidRequestException("Only clustering key
columns can be defined in CLUSTERING ORDER directive");
Review Comment:
We can improve the user experience by adding the names of the columns to the
exception message, similar to what Tomek did in our fork
--
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]
