Skip to content

Commit

Permalink
Materialized views incorrect quoting of UDF
Browse files Browse the repository at this point in the history
patch by Andres de la Peña, Jakub Zytka, Berenguer Blasi; reviewed by Andres de la Peña, Jakub Zytka for CASSANDRA-16836

Co-authored-by: Andres de la Peña <a.penya.garcia@gmail.com>
Co-authored-by: Jakub Zytka <jakub.zytka@datastax.com>
Co-authored-by: Berenguer Blasi <berenguerblasi@gmail.com>
  • Loading branch information
3 people committed Aug 13, 2021
1 parent ca6bb2a commit 99e1fcc
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 2 deletions.
Expand Up @@ -204,7 +204,7 @@ public AbstractType<?> getExactTypeIfKnown(String keyspace)

public String getText()
{
return name + terms.stream().map(Term.Raw::getText).collect(Collectors.joining(", ", "(", ")"));
return name.toCQLString() + terms.stream().map(Term.Raw::getText).collect(Collectors.joining(", ", "(", ")"));
}
}
}
18 changes: 18 additions & 0 deletions src/java/org/apache/cassandra/cql3/functions/FunctionName.java
Expand Up @@ -20,9 +20,13 @@
import com.google.common.base.Objects;

import org.apache.cassandra.config.SchemaConstants;
import org.apache.cassandra.cql3.ColumnIdentifier;

public final class FunctionName
{
// We special case the token function because that's the only function which name is a reserved keyword
private static final FunctionName TOKEN_FUNCTION_NAME = FunctionName.nativeFunction("token");

public final String keyspace;
public final String name;

Expand Down Expand Up @@ -79,4 +83,18 @@ public String toString()
{
return keyspace == null ? name : keyspace + "." + name;
}

/**
* Returns a string representation of the function name that is safe to use directly in CQL queries.
* If necessary, the name components will be double-quoted, and any quotes inside the names will be escaped.
*/
public String toCQLString()
{
String maybeQuotedName = equalsNativeFunction(TOKEN_FUNCTION_NAME)
? name
: ColumnIdentifier.maybeQuote(name);
return keyspace == null
? maybeQuotedName
: ColumnIdentifier.maybeQuote(keyspace) + '.' + maybeQuotedName;
}
}
122 changes: 121 additions & 1 deletion test/unit/org/apache/cassandra/cql3/ViewTest.java
Expand Up @@ -24,6 +24,8 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

import com.google.common.util.concurrent.Uninterruptibles;

import junit.framework.Assert;
Expand All @@ -44,6 +46,7 @@
import org.apache.cassandra.config.CFMetaData;
import org.apache.cassandra.config.ColumnDefinition;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.config.SchemaConstants;
import org.apache.cassandra.db.ColumnFamilyStore;
import org.apache.cassandra.db.Keyspace;
import org.apache.cassandra.db.SystemKeyspace;
Expand All @@ -52,6 +55,7 @@
import org.apache.cassandra.db.view.View;
import org.apache.cassandra.exceptions.SyntaxException;
import org.apache.cassandra.schema.KeyspaceParams;
import org.apache.cassandra.schema.SchemaKeyspace;
import org.apache.cassandra.service.ClientWarn;
import org.apache.cassandra.transport.ProtocolVersion;
import org.apache.cassandra.utils.FBUtilities;
Expand All @@ -71,6 +75,7 @@ public class ViewTest extends CQLTester
/** Latch used by {@link #testTruncateWhileBuilding()} Byteman injections. */
@SuppressWarnings("unused")
private static final CountDownLatch blockViewBuild = new CountDownLatch(1);
private static final AtomicInteger viewNameSeqNumber = new AtomicInteger();

private static final ProtocolVersion protocolVersion = ProtocolVersion.CURRENT;
private final List<String> views = new ArrayList<>();
Expand Down Expand Up @@ -1503,6 +1508,88 @@ public void viewOnCompactTableTest() throws Throwable
}
}

@Test
public void testFunctionInWhereClause() throws Throwable
{
// Native token function with lowercase, should be unquoted in the schema where clause
assertEmpty(testFunctionInWhereClause("CREATE TABLE %s (k bigint PRIMARY KEY, v int)",
null,
"CREATE MATERIALIZED VIEW %s AS" +
" SELECT * FROM %%s WHERE k = token(1) AND v IS NOT NULL " +
" PRIMARY KEY (v, k)",
"k = token(1) AND v IS NOT NULL",
"INSERT INTO %s(k, v) VALUES (0, 1)",
"INSERT INTO %s(k, v) VALUES (2, 3)"));

// Native token function with uppercase, should be unquoted and lowercased in the schema where clause
assertEmpty(testFunctionInWhereClause("CREATE TABLE %s (k bigint PRIMARY KEY, v int)",
null,
"CREATE MATERIALIZED VIEW %s AS" +
" SELECT * FROM %%s WHERE k = TOKEN(1) AND v IS NOT NULL" +
" PRIMARY KEY (v, k)",
"k = token(1) AND v IS NOT NULL",
"INSERT INTO %s(k, v) VALUES (0, 1)",
"INSERT INTO %s(k, v) VALUES (2, 3)"));

// UDF with lowercase name, shouldn't be quoted in the schema where clause
assertRows(testFunctionInWhereClause("CREATE TABLE %s (k int PRIMARY KEY, v int)",
"CREATE FUNCTION fun()" +
" CALLED ON NULL INPUT" +
" RETURNS int LANGUAGE java" +
" AS 'return 2;'",
"CREATE MATERIALIZED VIEW %s AS " +
" SELECT * FROM %%s WHERE k = fun() AND v IS NOT NULL" +
" PRIMARY KEY (v, k)",
"k = fun() AND v IS NOT NULL",
"INSERT INTO %s(k, v) VALUES (0, 1)",
"INSERT INTO %s(k, v) VALUES (2, 3)"), row(3, 2));

// UDF with uppercase name, should be quoted in the schema where clause
assertRows(testFunctionInWhereClause("CREATE TABLE %s (k int PRIMARY KEY, v int)",
"CREATE FUNCTION \"FUN\"()" +
" CALLED ON NULL INPUT" +
" RETURNS int" +
" LANGUAGE java" +
" AS 'return 2;'",
"CREATE MATERIALIZED VIEW %s AS " +
" SELECT * FROM %%s WHERE k = \"FUN\"() AND v IS NOT NULL" +
" PRIMARY KEY (v, k)",
"k = \"FUN\"() AND v IS NOT NULL",
"INSERT INTO %s(k, v) VALUES (0, 1)",
"INSERT INTO %s(k, v) VALUES (2, 3)"), row(3, 2));

// UDF with uppercase name conflicting with TOKEN keyword but not with native token function name,
// should be quoted in the schema where clause
assertRows(testFunctionInWhereClause("CREATE TABLE %s (k int PRIMARY KEY, v int)",
"CREATE FUNCTION \"TOKEN\"(x int)" +
" CALLED ON NULL INPUT" +
" RETURNS int" +
" LANGUAGE java" +
" AS 'return x;'",
"CREATE MATERIALIZED VIEW %s AS" +
" SELECT * FROM %%s WHERE k = \"TOKEN\"(2) AND v IS NOT NULL" +
" PRIMARY KEY (v, k)",
"k = \"TOKEN\"(2) AND v IS NOT NULL",
"INSERT INTO %s(k, v) VALUES (0, 1)",
"INSERT INTO %s(k, v) VALUES (2, 3)"), row(3, 2));

// UDF with lowercase name conflicting with both TOKEN keyword and native token function name,
// requires specifying the keyspace and should be quoted in the schema where clause
assertRows(testFunctionInWhereClause("CREATE TABLE %s (k int PRIMARY KEY, v int)",
"CREATE FUNCTION \"token\"(x int)" +
" CALLED ON NULL INPUT" +
" RETURNS int" +
" LANGUAGE java" +
" AS 'return x;'",
"CREATE MATERIALIZED VIEW %s AS" +
" SELECT * FROM %%s " +
" WHERE k = " + keyspace() + ".\"token\"(2) AND v IS NOT NULL" +
" PRIMARY KEY (v, k)",
"k = " + keyspace() + ".\"token\"(2) AND v IS NOT NULL",
"INSERT INTO %s(k, v) VALUES (0, 1)",
"INSERT INTO %s(k, v) VALUES (2, 3)"), row(3, 2));
}

/**
* Tests that truncating a table stops the ongoing builds of its materialized views,
* so they don't write into the MV data that has been truncated in the base table.
Expand Down Expand Up @@ -1557,4 +1644,37 @@ private static int runningCompactions()
{
return CompactionManager.instance.getPendingTasks() + CompactionManager.instance.getActiveCompactions();
}
}

private UntypedResultSet testFunctionInWhereClause(String createTableQuery,
String createFunctionQuery,
String createViewQuery,
String expectedSchemaWhereClause,
String... insertQueries) throws Throwable
{
createTable(createTableQuery);

execute("USE " + keyspace());
executeNet(protocolVersion, "USE " + keyspace());

if (createFunctionQuery != null)
{
execute(createFunctionQuery);
}

String viewName = "view_" + viewNameSeqNumber.getAndIncrement();
createView(viewName, createViewQuery);

// Test the where clause stored in system_schema.views
String schemaQuery = String.format("SELECT where_clause FROM %s.%s WHERE keyspace_name = ? AND view_name = ?",
SchemaConstants.SCHEMA_KEYSPACE_NAME,
SchemaKeyspace.VIEWS);
assertRows(execute(schemaQuery, keyspace(), viewName), row(expectedSchemaWhereClause));

for (String insert : insertQueries)
{
execute(insert);
}

return execute("SELECT * FROM " + viewName);
}
}

0 comments on commit 99e1fcc

Please sign in to comment.