From 1eccb2bc1ff69817b2fc8d16a4707b64d8b514e7 Mon Sep 17 00:00:00 2001 From: "ningzi.zhan" Date: Thu, 8 Jun 2023 14:59:17 -0700 Subject: [PATCH] Fix error message handling when trying to use CLUSTERING ORDER with non-clustering column Patch by Ningzi Zhan and Tomasz Lasica; reviewed by brandonwilliams, edimitrova and Maxwell Guo for CASSANDRA-17818 Co-Authored-By: Tomek Lasica --- CHANGES.txt | 1 + .../cql3/statements/CreateTableStatement.java | 12 +++- .../schema/CreateTableValidationTest.java | 59 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index cd7fe124ea..942651a37e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.11.16 + * Fix error message handling when trying to use CLUSTERING ORDER with non-clustering column (CASSANDRA-17818 * Add keyspace and table name to exception message during ColumnSubselection deserialization (CASSANDRA-18346) * Remove unnecessary String.format invocation in QueryProcessor when getting a prepared statement from cache (CASSANDRA-17202) Merged from 3.0: diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateTableStatement.java index 90ed288f1e..ba6a153691 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/CreateTableStatement.java @@ -20,6 +20,8 @@ package org.apache.cassandra.cql3.statements; import java.nio.ByteBuffer; import java.util.*; import java.util.regex.Pattern; +import java.util.stream.Collectors; + import com.google.common.collect.HashMultiset; import com.google.common.collect.Multiset; import org.apache.commons.lang3.StringUtils; @@ -342,8 +344,14 @@ public class CreateTableStatement extends SchemaAlteringStatement // If we give a clustering order, we must explicitly do so for all aliases and in the order of the PK if (!properties.definedOrdering.isEmpty()) { - if (properties.definedOrdering.size() > columnAliases.size()) - throw new InvalidRequestException("Only clustering key columns can be defined in CLUSTERING ORDER directive"); + List nonClusterColumn = properties.definedOrdering.keySet().stream() + .filter((id) -> !columnAliases.contains(id)) + .collect(Collectors.toList()); + + if (!nonClusterColumn.isEmpty()) + { + throw new InvalidRequestException("Only clustering key columns can be defined in CLUSTERING ORDER directive: " + nonClusterColumn + " are not clustering columns"); + } int i = 0; for (ColumnIdentifier id : properties.definedOrdering.keySet()) diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index 9708552382..7ff313c425 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -20,8 +20,11 @@ package org.apache.cassandra.schema; import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.exceptions.InvalidRequestException; + import org.junit.Test; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.Assert.fail; public class CreateTableValidationTest extends CQLTester @@ -48,4 +51,60 @@ public class CreateTableValidationTest extends CQLTester // sanity check createTable("CREATE TABLE %s (a int PRIMARY KEY, b int) WITH bloom_filter_fp_chance = 0.1"); } + + @Test + public void testCreateTableOnSelectedClusteringColumn() + { + createTable("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC);"); + } + + @Test + public void testCreateTableOnAllClusteringColumns() + { + 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 testCreateTableErrorOnNonClusteringKey() + { + String expectedMessage = "Only clustering key columns can be defined in CLUSTERING ORDER directive"; + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC, v ASC);", + expectedMessage+": [v]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (v ASC);", + expectedMessage+": [v]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk ASC);", + expectedMessage+": [pk]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk ASC, ck1 DESC);", + expectedMessage+": [pk]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC, pk DESC);", + expectedMessage+": [pk]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk DESC, v DESC);", + expectedMessage+": [pk, v]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk DESC, v DESC, ck1 DESC);", + expectedMessage+": [pk, v]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, v ASC);", + expectedMessage+": [v]"); + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (v ASC, ck1 DESC);", + expectedMessage+": [v]"); + } + + @Test + public void testCreateTableInWrongOrder() + { + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck2 ASC, ck1 DESC);", + "The order of columns in the CLUSTERING ORDER directive must be the one of the clustering key"); + } + + @Test + public void testCreateTableWithMissingClusteringColumn() + { + expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck2 ASC);", + "Missing CLUSTERING ORDER for column ck1"); + } + + private void expectedFailure(String statement, String errorMsg) + { + assertThatExceptionOfType(InvalidRequestException.class) + .isThrownBy(() -> createTableMayThrow(statement)) .withMessageContaining(errorMsg); + + } }