From 75194201f1f06d120f246f6fad025ca5f672943d Mon Sep 17 00:00:00 2001 From: Bernardo Botella Corbi Date: Mon, 10 Oct 2022 09:08:16 -0700 Subject: [PATCH] Fix quoting in toCqlString methods of UDTs and aggregates patch by Bernardo Botella Corbi, reviewed by Stefan Miklosovic, Benjamin Lerer and Yifan Cai for CASSANDRA-17918 --- CHANGES.txt | 1 + .../cassandra/cql3/functions/UDAggregate.java | 4 +- .../apache/cassandra/db/marshal/UserType.java | 2 +- .../statements/DescribeStatementTest.java | 130 ++++++++++++++++++ 4 files changed, 134 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index ac053c3f5b..f6c5cfc757 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.10 + * Fix quoting in toCqlString methods of UDTs and aggregates (CASSANDRA-17918) * NPE when deserializing malformed collections from client (CASSANDRA-18505) * Improve 'Not enough space for compaction' logging messages (CASSANDRA-18260) * Incremental repairs fail on mixed IPv4/v6 addresses serializing SyncRequest (CASSANDRA-18474) diff --git a/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java b/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java index b201f09f9d..b7648354b8 100644 --- a/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java +++ b/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java @@ -349,7 +349,7 @@ public class UDAggregate extends AbstractFunction implements AggregateFunction, .newLine() .increaseIndent() .append("SFUNC ") - .append(stateFunction().name().name) + .appendQuotingIfNeeded(stateFunction().name().name) .newLine() .append("STYPE ") .append(toCqlString(stateType())); @@ -357,7 +357,7 @@ public class UDAggregate extends AbstractFunction implements AggregateFunction, if (finalFunction() != null) builder.newLine() .append("FINALFUNC ") - .append(finalFunction().name().name); + .appendQuotingIfNeeded(finalFunction().name().name); if (initialCondition() != null) builder.newLine() diff --git a/src/java/org/apache/cassandra/db/marshal/UserType.java b/src/java/org/apache/cassandra/db/marshal/UserType.java index 29afad9583..64bdcddda7 100644 --- a/src/java/org/apache/cassandra/db/marshal/UserType.java +++ b/src/java/org/apache/cassandra/db/marshal/UserType.java @@ -495,7 +495,7 @@ public class UserType extends TupleType implements SchemaElement builder.append(",") .newLine(); - builder.append(fieldNameAsString(i)) + builder.appendQuotingIfNeeded(fieldNameAsString(i)) .append(' ') .append(fieldType(i)); } diff --git a/test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java b/test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java index 196b34e5c5..940e7b6760 100644 --- a/test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java +++ b/test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java @@ -711,17 +711,49 @@ public class DescribeStatementTest extends CQLTester executeDescribeNet(KEYSPACE_PER_TEST, "DROP MATERIALIZED VIEW " + table + "_view"); execute("DROP TABLE " + KEYSPACE_PER_TEST + "." + table); + String aggregationFunctionName = KEYSPACE_PER_TEST + ".\"token\""; + String aggregationName = KEYSPACE_PER_TEST + ".\"aggregate\""; + createFunction(KEYSPACE_PER_TEST, + "int, int", + "CREATE FUNCTION " + aggregationFunctionName + " (\"token\" int, add_to int) " + + "CALLED ON NULL INPUT " + + "RETURNS int " + + "LANGUAGE java " + + "AS 'return token + add_to;'"); + + createAggregate(KEYSPACE_PER_TEST, + "int", + format("CREATE AGGREGATE " + aggregationName + "(int) " + + "SFUNC %s " + + "STYPE int " + + "INITCOND 42", + shortFunctionName(aggregationFunctionName))); + + String functionCreate = executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE FUNCTION " + aggregationFunctionName).all().get(0).getString("create_statement"); + String aggregateCreate = executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE AGGREGATE " + aggregationName).all().get(0).getString("create_statement"); + + execute("DROP AGGREGATE " + aggregationName); + execute("DROP FUNCTION " + aggregationFunctionName); + executeNet(output); executeNet(mvCreateView); + executeNet(functionCreate); + executeNet(aggregateCreate); String output2 = executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE TABLE " + table + withInternals).all().get(0).getString("create_statement"); String mvCreateView2 = executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE MATERIALIZED VIEW " + table + "_view").all().get(0).getString("create_statement"); + String functionCreate2 = executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE FUNCTION " + aggregationFunctionName).all().get(0).getString("create_statement"); + String aggregateCreate2 = executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE AGGREGATE " + aggregationName).all().get(0).getString("create_statement"); assertEquals(output, output2); assertEquals(mvCreateView, mvCreateView2); + assertEquals(functionCreate, functionCreate2); + assertEquals(aggregateCreate, aggregateCreate2); execute("INSERT INTO " + KEYSPACE_PER_TEST + "." + table + " (key) VALUES (1)"); executeDescribeNet(KEYSPACE_PER_TEST, "DROP MATERIALIZED VIEW " + table + "_view"); + executeDescribeNet(KEYSPACE_PER_TEST, "DROP AGGREGATE " + aggregationName); + executeDescribeNet(KEYSPACE_PER_TEST, "DROP FUNCTION " + aggregationFunctionName); } } @@ -757,6 +789,104 @@ public class DescribeStatementTest extends CQLTester row(KEYSPACE_PER_TEST, "index", indexWithOptions, expectedIndexStmtWithOptions)); } + @Test + public void testUsingReservedInCreateType() throws Throwable + { + String type = createType(KEYSPACE_PER_TEST, "CREATE TYPE %s (\"token\" text, \"desc\" text);"); + assertRowsNet(executeDescribeNet(KEYSPACE_PER_TEST, "DESCRIBE TYPE " + type), + row(KEYSPACE_PER_TEST, "type", type, "CREATE TYPE " + KEYSPACE_PER_TEST + "." + type + " (\n" + + " \"token\" text,\n" + + " \"desc\" text\n" + + ");")); + } + + @Test + public void testDescMaterializedViewShouldNotOmitQuotations() throws Throwable + { + try{ + execute("CREATE KEYSPACE testWithKeywords WITH REPLICATION = {'class' : 'SimpleStrategy', 'replication_factor' : 1};"); + execute("CREATE TABLE testWithKeywords.users_mv (username varchar, password varchar, gender varchar, session_token varchar, " + + "state varchar, birth_year bigint, \"token\" text, PRIMARY KEY (\"token\"));"); + execute("CREATE MATERIALIZED VIEW testWithKeywords.users_by_state AS SELECT * FROM testWithKeywords.users_mv " + + "WHERE STATE IS NOT NULL AND \"token\" IS NOT NULL PRIMARY KEY (state, \"token\")"); + + final String expectedOutput = "CREATE MATERIALIZED VIEW testwithkeywords.users_by_state AS\n" + + " SELECT *\n" + + " FROM testwithkeywords.users_mv\n" + + " WHERE state IS NOT NULL AND \"token\" IS NOT NULL\n" + + " PRIMARY KEY (state, \"token\")\n" + + " WITH CLUSTERING ORDER BY (\"token\" ASC)\n" + + " AND " + mvParametersCql(); + + testDescribeMaterializedView("testWithKeywords", "users_by_state", row("testwithkeywords", "materialized_view", "users_by_state", expectedOutput)); + } + finally + { + execute("DROP KEYSPACE IF EXISTS testWithKeywords"); + } + } + + @Test + public void testDescFunctionAndAggregateShouldNotOmitQuotations() throws Throwable + { + + final String functionName = KEYSPACE_PER_TEST + ".\"token\""; + + createFunctionOverload(functionName, + "int, ascii", + "CREATE FUNCTION " + functionName + " (\"token\" int, other_in ascii) " + + "RETURNS NULL ON NULL INPUT " + + "RETURNS text " + + "LANGUAGE java " + + "AS 'return \"Hello World\";'"); + + for (String describeKeyword : new String[]{"DESCRIBE", "DESC"}) + { + + assertRowsNet(executeDescribeNet(describeKeyword + " FUNCTION " + functionName), + row(KEYSPACE_PER_TEST, + "function", + shortFunctionName(functionName) + "(int, ascii)", + "CREATE FUNCTION " + functionName + "(\"token\" int, other_in ascii)\n" + + " RETURNS NULL ON NULL INPUT\n" + + " RETURNS text\n" + + " LANGUAGE java\n" + + " AS $$return \"Hello World\";$$;")); + } + + final String aggregationFunctionName = KEYSPACE_PER_TEST + ".\"token\""; + final String aggregationName = KEYSPACE_PER_TEST + ".\"token\""; + createFunctionOverload(aggregationName, + "int, int", + "CREATE FUNCTION " + aggregationFunctionName + " (\"token\" int, add_to int) " + + "CALLED ON NULL INPUT " + + "RETURNS int " + + "LANGUAGE java " + + "AS 'return token + add_to;'"); + + + String aggregate = createAggregate(KEYSPACE_PER_TEST, + "int", + format("CREATE AGGREGATE %%s(int) " + + "SFUNC %s " + + "STYPE int " + + "INITCOND 42", + shortFunctionName(aggregationFunctionName))); + + + for (String describeKeyword : new String[]{"DESCRIBE", "DESC"}) + { + assertRowsNet(executeDescribeNet(describeKeyword + " AGGREGATE " + aggregate), + row(KEYSPACE_PER_TEST, + "aggregate", + shortFunctionName(aggregate) + "(int)", + "CREATE AGGREGATE " + aggregate + "(int)\n" + + " SFUNC " + shortFunctionName(aggregationName) + "\n" + + " STYPE int\n" + + " INITCOND 42;")); + } + } + private static String allTypesTable() { return "CREATE TABLE test.has_all_types (\n" +