From b9ad8c012ac3e2493db0e7751227321940136518 Mon Sep 17 00:00:00 2001 From: Ruslan Fomkin Date: Fri, 7 Mar 2025 11:48:09 +0100 Subject: [PATCH 1/6] CNDB-12683 try to validate table name length for not-internal --- .../cql3/statements/schema/CreateTableStatement.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java index 5ea443220634..7ab0ec6d604d 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -41,6 +41,7 @@ import org.apache.cassandra.guardrails.Guardrails; import org.apache.cassandra.guardrails.UserKeyspaceFilter; import org.apache.cassandra.guardrails.UserKeyspaceFilterProvider; +import org.apache.cassandra.io.sstable.Component; import org.apache.cassandra.schema.*; import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff; import org.apache.cassandra.service.ClientState; @@ -108,6 +109,12 @@ public boolean isCompactStorage() public void validate(QueryState state) { super.validate(state); + logger.debug("Table {}, internal state {}", tableName, state.getClientState().isInternal); + + int separatorLength = 1; + if (!state.getClientState().isInternal && tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) + throw ire("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s)", + SchemaConstants.NAME_LENGTH - separatorLength, keyspaceName.length(), tableName.length(), tableName); // Some tools use CreateTableStatement, and the guardrails below both don't make too much sense for tools and // require the server to be initialized, so skipping them if it isn't. From d8b10aae09e9668e0cce2cdbe004f114363d8459 Mon Sep 17 00:00:00 2001 From: Ruslan Fomkin Date: Tue, 11 Mar 2025 10:43:19 +0100 Subject: [PATCH 2/6] CNDB-12683 test table name length validation for new table --- .../schema/CreateTableStatement.java | 5 ++--- .../schema/CreateTableValidationTest.java | 21 ++++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java index 7ab0ec6d604d..ba1f5e24696b 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -41,7 +41,6 @@ import org.apache.cassandra.guardrails.Guardrails; import org.apache.cassandra.guardrails.UserKeyspaceFilter; import org.apache.cassandra.guardrails.UserKeyspaceFilterProvider; -import org.apache.cassandra.io.sstable.Component; import org.apache.cassandra.schema.*; import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff; import org.apache.cassandra.service.ClientState; @@ -113,8 +112,8 @@ public void validate(QueryState state) int separatorLength = 1; if (!state.getClientState().isInternal && tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) - throw ire("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s)", - SchemaConstants.NAME_LENGTH - separatorLength, keyspaceName.length(), tableName.length(), tableName); + throw ire("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s.%s)", + SchemaConstants.NAME_LENGTH - separatorLength, keyspaceName.length(), tableName.length(), keyspaceName, tableName); // Some tools use CreateTableStatement, and the guardrails below both don't make too much sense for tools and // require the server to be initialized, so skipping them if it isn't. diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index 8903640f2fa7..284659d4306c 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -18,6 +18,7 @@ */ package org.apache.cassandra.schema; +import com.datastax.driver.core.exceptions.InvalidQueryException; import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.UntypedResultSet; import org.apache.cassandra.cql3.functions.types.ParseUtils; @@ -104,7 +105,25 @@ public void testCreateTableWithMissingClusteringColumn() } @Test - public void testCreatingTableWithLongName() throws Throwable + public void failCreatingNewTableWithLongName() + { + String keyspace = "38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch"; + String table = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"; + + execute(String.format("CREATE KEYSPACE \"%s\" with replication = " + + "{ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }", + keyspace)); + assertThatExceptionOfType(InvalidQueryException.class) + .isThrownBy(() -> executeNet(String.format("CREATE TABLE \"%s\".%s (" + + "key int PRIMARY KEY," + + "val int)", + keyspace, table))) + .withMessageContaining(String.format("Keyspace and table names combined shouldn't be more than 221 characters long (got keyspace of %s chars and table of %s chars for %s.%s)", + keyspace.length(), table.length(), keyspace, table)); + } + + @Test + public void testCreatingInternalTableWithLongName() throws Throwable { String keyspace = "\"38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch\""; String table = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"; From c43325ac270d00b81c693bb893de38845d9f4987 Mon Sep 17 00:00:00 2001 From: Ruslan Fomkin Date: Tue, 11 Mar 2025 12:43:30 +0100 Subject: [PATCH 3/6] Fix test of failing new long name table creation --- .../cassandra/schema/CreateTableValidationTest.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index 284659d4306c..a30cdc9bf086 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -107,19 +107,14 @@ public void testCreateTableWithMissingClusteringColumn() @Test public void failCreatingNewTableWithLongName() { - String keyspace = "38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch"; String table = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"; - - execute(String.format("CREATE KEYSPACE \"%s\" with replication = " + - "{ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }", - keyspace)); assertThatExceptionOfType(InvalidQueryException.class) .isThrownBy(() -> executeNet(String.format("CREATE TABLE \"%s\".%s (" + "key int PRIMARY KEY," + "val int)", - keyspace, table))) + KEYSPACE, table))) .withMessageContaining(String.format("Keyspace and table names combined shouldn't be more than 221 characters long (got keyspace of %s chars and table of %s chars for %s.%s)", - keyspace.length(), table.length(), keyspace, table)); + KEYSPACE.length(), table.length(), KEYSPACE, table)); } @Test From 151a40c74455035219d3d89e3c4055521b1ba270 Mon Sep 17 00:00:00 2001 From: Ruslan Fomkin Date: Tue, 11 Mar 2025 12:44:25 +0100 Subject: [PATCH 4/6] Break long table name creation for any table Needed to verify test in CNDB --- .../cassandra/cql3/statements/schema/CreateTableStatement.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java index ba1f5e24696b..27551ddbbd95 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -111,7 +111,7 @@ public void validate(QueryState state) logger.debug("Table {}, internal state {}", tableName, state.getClientState().isInternal); int separatorLength = 1; - if (!state.getClientState().isInternal && tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) + if (tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) throw ire("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s.%s)", SchemaConstants.NAME_LENGTH - separatorLength, keyspaceName.length(), tableName.length(), keyspaceName, tableName); From 9123c19e29156dcfe86ce9d3f50b471432c0e767 Mon Sep 17 00:00:00 2001 From: Ruslan Fomkin Date: Tue, 11 Mar 2025 14:24:20 +0100 Subject: [PATCH 5/6] Revert and validate only on new tables --- .../cassandra/cql3/statements/schema/CreateTableStatement.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java index 27551ddbbd95..ba1f5e24696b 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -111,7 +111,7 @@ public void validate(QueryState state) logger.debug("Table {}, internal state {}", tableName, state.getClientState().isInternal); int separatorLength = 1; - if (tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) + if (!state.getClientState().isInternal && tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) throw ire("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s.%s)", SchemaConstants.NAME_LENGTH - separatorLength, keyspaceName.length(), tableName.length(), keyspaceName, tableName); From 0d71ddedd35b067f1570b8e2c1ad57fd16716a18 Mon Sep 17 00:00:00 2001 From: Ruslan Fomkin Date: Wed, 12 Mar 2025 11:47:03 +0100 Subject: [PATCH 6/6] CNDB-12683 simplify the length check and cleanup --- .../cql3/statements/schema/CreateTableStatement.java | 6 ++---- .../apache/cassandra/schema/CreateTableValidationTest.java | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java index ba1f5e24696b..631e123ef851 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -108,12 +108,10 @@ public boolean isCompactStorage() public void validate(QueryState state) { super.validate(state); - logger.debug("Table {}, internal state {}", tableName, state.getClientState().isInternal); - int separatorLength = 1; - if (!state.getClientState().isInternal && tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length() - separatorLength) + if (!state.getClientState().isInternal && tableName.length() > SchemaConstants.NAME_LENGTH - keyspaceName.length()) throw ire("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s.%s)", - SchemaConstants.NAME_LENGTH - separatorLength, keyspaceName.length(), tableName.length(), keyspaceName, tableName); + SchemaConstants.NAME_LENGTH, keyspaceName.length(), tableName.length(), keyspaceName, tableName); // Some tools use CreateTableStatement, and the guardrails below both don't make too much sense for tools and // require the server to be initialized, so skipping them if it isn't. diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index a30cdc9bf086..7ccf7cbf8e4e 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -113,8 +113,8 @@ public void failCreatingNewTableWithLongName() "key int PRIMARY KEY," + "val int)", KEYSPACE, table))) - .withMessageContaining(String.format("Keyspace and table names combined shouldn't be more than 221 characters long (got keyspace of %s chars and table of %s chars for %s.%s)", - KEYSPACE.length(), table.length(), KEYSPACE, table)); + .withMessageContaining(String.format("Keyspace and table names combined shouldn't be more than %s characters long (got keyspace of %s chars and table of %s chars for %s.%s)", + SchemaConstants.NAME_LENGTH, KEYSPACE.length(), table.length(), KEYSPACE, table)); } @Test