You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Andres de la Peña (Jira)" <ji...@apache.org> on 2021/05/18 14:03:00 UTC

[jira] [Commented] (CASSANDRA-16510) Make ReadCommand::toCQLString return valid CQL

    [ https://issues.apache.org/jira/browse/CASSANDRA-16510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17346921#comment-17346921 ] 

Andres de la Peña commented on CASSANDRA-16510:
-----------------------------------------------

The proposed [PR|https://github.com/apache/cassandra/pull/1013] makes sure that {{ReadCommand::toCQLString}} always returns valid CQL, solving the problems mentioned in the description and some other minor problems. This includes the {{toCQLString}}/{{toString}} of some command components, such as {{Clustering}}, {{DataRange}}, {{DecoratedKey}}, {{Slices}}, etc.

The trickiest part is {{ALLOW FILTERING}}, since it's not easy to determine whether a command strictly needs filtering. This check is done during parsing and the concept doesn't exist internally. For that I have just changed the implementations of {{toCQLString}} to simply always add {{ALLOW FILTERING}}. This is not ideal but IMO it's correct and consistent with the idea that filtering is always allowed internally. I think that SAI will introduce changes around AF that would ease to determine whether the command requires filtering.

Since this is on the verge of being a bug fix I'm not sure whether this patch should be applied to previous branches.

CI with repeated runs looks good:
* [CircleCI j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/458/workflows/0d6acc32-3813-4d64-a41e-4078b2ddcdc7]
* [CircleCI j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/458/workflows/0d0d5d1e-fa0e-4343-b787-48f575c9f934]

> Make ReadCommand::toCQLString return valid CQL
> ----------------------------------------------
>
>                 Key: CASSANDRA-16510
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-16510
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: CQL/Syntax, Observability/Logging
>            Reporter: Andres de la Peña
>            Assignee: Andres de la Peña
>            Priority: Normal
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> The method {{ReadCommand::toCQLString}} doesn't always return queries that are valid CQL. For example, queries for a table without clustering columns always add the {{WHERE}} keyword even if there isn't a restriction following it:
> {code:java}
> CREATE TABLE t (k int PRIMARY KEY, v int);
> SELECT * FROM t; 
> -- toCQLString 3.0:        SELECT * FROM k.t WHERE () = () LIMIT 100
> -- toCQLString 3.11/trunk: SELECT * FROM k.t WHERE  LIMIT 100
> {code}
> Also, case sensitive names and text values are not properly quoted:
> {code:java}
> CREATE TABLE "T" ("K" text, "C" text, "V" text, PRIMARY KEY("K", "C"));
> SELECT * FROM "T" WHERE "K" = 'a' AND "C" = 'b' AND "V" = 'c' ALLOW FILTERING;
> -- toCQLString 3.0/3.11/trunk: SELECT * FROM k.T WHERE K = a AND V = c AND (C) = (b) LIMIT 100
> {code}
> Similar problems can be found on selections of collection items:
> {code:java}
> CREATE TABLE k.t (k int, c int, s set<text>,  m map<text, text>, PRIMARY KEY(k, c));
> SELECT s['a'] FROM t;
> -- toCQLString trunk: SELECT s[a] FROM k.t LIMIT 100
> SELECT * FROM t WHERE m['a'] = 'b' ALLOW FILTERING;
> -- toCQLString 3.0/3.11/trunk: SELECT * FROM k.t WHERE m[a] = b LIMIT 100
> {code}
> Some similar problems with more impact than the described above are already being addressed in CASSANDRA-16483 and CASSANDRA-16479.
> I think we should add more exhaustive JUnit tests for {{ReadCommand::toCQLString}}, making sure that its output is parseable and equivalent to the original command. Also, we probably should make sure that we have {{toCQLString}} methods in all the query components that are used by {{ReadCommand::toCQLString}} (selection, filters, etc.). This way we can use them instead of generic {{toString}} methods, making it clear that they are supposed to return valid CQL.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org