CASSANDRA-17818-3.11: Fix error message handling when trying to use CLUSTERING ORDER with non-clustering column #2398
Conversation
| { | ||
| if (!columnAliases.contains(orderId)) | ||
| { | ||
| throw new InvalidRequestException("Only clustering key columns can be defined in CLUSTERING ORDER directive"); |
There was a problem hiding this 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
| @@ -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() | |||
There was a problem hiding this 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.
There was a problem hiding this comment.
+1 with @ekaterinadimitrova2
add column name to error message
add column name to error message
| @@ -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() | |||
There was a problem hiding this comment.
+1 with @ekaterinadimitrova2
| } | ||
|
|
||
| @Test | ||
| public void testCreateTableInWrongOrdering() |
There was a problem hiding this comment.
| public void testCreateTableInWrongOrdering() | |
| public void testCreateTableInWrongOrder() |
to be corrected on commit
| } | ||
|
|
||
| @Test | ||
| public void testCreateTableWithMissingClusterColumn() |
There was a problem hiding this comment.
| public void testCreateTableWithMissingClusterColumn() | |
| public void testCreateTableWithMissingClusteringColumn() |
to be fixed on commit
| createTable("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC);"); | ||
| } | ||
| @Test | ||
| public void testCreateTableErrorOnNonClusterKey() |
There was a problem hiding this comment.
| public void testCreateTableErrorOnNonClusterKey() | |
| public void testCreateTableErrorOnNonClusteringKey() |
to be corrected on commit
| } | ||
|
|
||
| @Test | ||
| public void testCreateTableOnAllClusterColumns() |
There was a problem hiding this comment.
| public void testCreateTableOnAllClusterColumns() | |
| public void testCreateTableOnAllClusterColumns() |
to be corrected on commit
| @@ -48,4 +51,60 @@ 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 testCreateTableOnSelectedClusterColumn() | |||
There was a problem hiding this comment.
| public void testCreateTableOnSelectedClusterColumn() | |
| public void testCreateTableOnSelectedClusteringColumn() |
to be corrected on commit
|
|
||
| if (!nonClusterColumn.isEmpty()) | ||
| { | ||
| throw new InvalidRequestException("Only clustering key columns can be defined in CLUSTERING ORDER directive: " + nonClusterColumn + " are not clustering column"); |
There was a problem hiding this comment.
| throw new InvalidRequestException("Only clustering key columns can be defined in CLUSTERING ORDER directive: " + nonClusterColumn + " are not clustering column"); | |
| throw new InvalidRequestException("Only clustering key columns can be defined in CLUSTERING ORDER directive: " + nonClusterColumn + " are not clustering columns"); |
to be corrected on commit
Fix error message handling when trying to use CLUSTERING ORDER with non-clustering column
Patch: Throw error message after finding there are non-cluster columns in order definition.
patch by
The Cassandra Jira