You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/10/10 23:03:05 UTC
[GitHub] [cassandra] bbotella opened a new pull request, #1907: Fix double quote parser
bbotella opened a new pull request, #1907:
URL: https://github.com/apache/cassandra/pull/1907
patch by Bernardo Botella Corbi
CASSANDRA-17918
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic closed pull request #1907: Fix double quote parser
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #1907: Fix double quote parser
URL: https://github.com/apache/cassandra/pull/1907
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on a diff in pull request #1907: Fix double quote parser
Posted by GitBox <gi...@apache.org>.
yifan-c commented on code in PR #1907:
URL: https://github.com/apache/cassandra/pull/1907#discussion_r1034185853
##########
test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java:
##########
@@ -757,6 +757,103 @@ public void testDescribeWithCustomIndex() throws Throwable
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 testUsingReservedInMaterializedView() 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 testUsingReservedDescribeFunctionAndAggregate() 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;'");
Review Comment:
nit: align the parameters
##########
test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java:
##########
@@ -757,6 +757,103 @@ public void testDescribeWithCustomIndex() throws Throwable
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 testUsingReservedInMaterializedView() 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 testUsingReservedDescribeFunctionAndAggregate() 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\";'");
Review Comment:
nit: align the parameters
##########
test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java:
##########
@@ -757,6 +757,103 @@ public void testDescribeWithCustomIndex() throws Throwable
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 testUsingReservedInMaterializedView() 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
Review Comment:
nit: put `finally` in a new line.
##########
test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java:
##########
@@ -757,6 +757,103 @@ public void testDescribeWithCustomIndex() throws Throwable
row(KEYSPACE_PER_TEST, "index", indexWithOptions, expectedIndexStmtWithOptions));
}
+ @Test
+ public void testUsingReservedInCreateType() throws Throwable
Review Comment:
nit: maybe call it `testDescTypeShouldNotOmitQuotations` or similar. Similarly, change the names of the other newly added test cases.
The reason is that the field names are quoted and they are not really "reserved". The test is to verify the output of `DESC` should preserve the quotation marks.
##########
test/unit/org/apache/cassandra/cql3/statements/DescribeStatementTest.java:
##########
@@ -757,6 +757,103 @@ public void testDescribeWithCustomIndex() throws Throwable
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 testUsingReservedInMaterializedView() 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 testUsingReservedDescribeFunctionAndAggregate() 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)));
Review Comment:
nit: align those lines too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org