[
https://issues.apache.org/jira/browse/CASSANDRA-16483?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17296259#comment-17296259
]
Andres de la Peña edited comment on CASSANDRA-16483 at 3/5/21, 6:27 PM:
------------------------------------------------------------------------
As promised, the proposed patch adds a new {{ColumnFilter#toCQLString}} method,
so we can keep the changes in {{ColumnFilter#toString}} that were introduced by
CASSANDRA-16415. The behaviour of the new method is tested in
{{ColumnFilterTest}}, and the {{TestCQLSlowQuery}} dtests are extended to
include column selections.
As for producing valid CQL in {{AbstractReadQuery.toCQLString}}, I have
replaced [the illegal
strings|https://github.com/apache/cassandra/blob/b063f30f51e61d6298e79b43f7eb99b581bbec14/src/java/org/apache/cassandra/db/filter/ColumnFilter.java#L489-L494]
that were produced before CASSANDRA-16415 by just {{*}}. This is not ideal but
I guess it's acceptable since we can't know what primary key columns were
selected by the original query, nor distinguish between queries asking for all
columns and queries asking only for primary key columns. I've also included
quoting for column names needing escaping.
It's worth mentioning that {{AbstractReadQuery.toCQLString}} can still produce
illegal query strings for other parts of the query. For example, the {{WHERE}}
keyword seems to be always included even if there isn't an expression following
it. Also, the filtering expressions don't properly quote column names nor
values. I think we should address those problems across several {{toCQLString}}
implementations in a separate ticket, probably without blocking 4.0.
was (Author: adelapena):
As promised, the proposed patch adds a new {{ColumnFilter#toCQLString}} method,
so we can keep the changes in {{ColumnFilter#toString}} that were introduced by
CASSANDRA-16415. The behaviour of the new method is tested in
{{ColumnFilterTest}}, and the {{TestCQLSlowQuery}} dtests are extended to
include column selections.
As for producing valid CQL in {{AbstractReadQuery.toCQLString}}, I have
replaced [the illegal
strings|https://github.com/apache/cassandra/blob/b063f30f51e61d6298e79b43f7eb99b581bbec14/src/java/org/apache/cassandra/db/filter/ColumnFilter.java#L489-L494]
that were produced before CASSANDRA-16415 by just {{*}}. This is not ideal but
I guess it's acceptable since we can't know what primary key columns were
selected by the original query, nor distinguish between queries asking for all
columns and queries asking only for primary key columns. I've also included
quoting for column names needing escaping.
It's worth mentioning that {{AbstractReadQuery.toCQLString}} can still produce
illegal query strings for other parts of the query. For example, the {{WHERE}}
keyword seems to be always included even if there isn't an expression following
it. Also, the filtering expressions don't properly quote column names nor
values. I think we should address those problems across several
{{{{toCQLString}}}} implementations in a separate ticket, probably without
blocking 4.0.
> ColumnFilter::toString doesn't return a valid CQL fragment
> ----------------------------------------------------------
>
> Key: CASSANDRA-16483
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16483
> Project: Cassandra
> Issue Type: Bug
> Components: Observability/Logging
> Reporter: Sam Tunnicliffe
> Assignee: Andres de la Peña
> Priority: Normal
> Fix For: 4.0-beta
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> This was changed in CASSANDRA-16415 to include indications about queried vs
> fetched reagular & static columns. However, the result is used by
> {{AbstractReadQuery::toCQLString}}, which causes it to produce an illegal
> query string.
> This breaks a couple of dtests because they're looking for CQL strings in
> logs, which are no longer found:
> *
> {{upgrade_tests/paging_test.py::TestPagingWithDeletions::test_failure_threshold_deletions}}
> * {{cql_test.py::TestCQLSlowQuery}} has a couple of failing tests,
> {{test_local_query/test_remote_query}}
> We should also check audit and fql logs (and any other place where
> {{toCQLString}} is used.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
