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/11/29 00:36:38 UTC

[GitHub] [cassandra] yifan-c commented on a diff in pull request #1907: Fix double quote parser

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