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/03/05 18:28:00 UTC
[jira] [Comment Edited] (CASSANDRA-16483) ColumnFilter::toString
doesn't return a valid CQL fragment
[ 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 it 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: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org