CNDB-15760: Fix AbstractReadQuery.toCQLString

[adelapena]

What is the issue

Testing of CNDB-15280 has revealed that the output of AbstractReadQuery.toCQLString is frequently invalid CQL.

What does this PR fix and why was it fixed

This PR fixes AbstractReadQuery.toCQLString` to produce output that is as close to valid CQL as possible.

The output cannot always be entirely valid CQL because:

• MultiRangeReadCommand doesn't translate to any semantically valid CQL due to the multiple token ranges, although we can still produce syntactically valid CQL.
• Long queried column values are truncated to save space and be less distracting.

This is closely related to CNDB-15280, which testing exposes most of the issues fixed here, so both issues should be blocked on each other.

[github-actions]

Checklist before you submit for review

• This PR adheres to the Definition of Done
• Make sure there is a PR in the CNDB project updating the Converged Cassandra version
• Use NoSpamLogger for log lines that may appear frequently in the logs
• Verify test results on Butler
• Test coverage for new/modified code is > 80%
• Proper code formatting
• Proper title for each commit staring with the project-issue number, like CNDB-1234
• Each commit has a meaningful description
• Each commit is not very long and contains related changes
• Renames, moves and reformatting are in distinct commits
• All new files should contain the DataStax copyright header instead of the Apache License one

[k-rus]

1 New issue

@adelapena can you look and fix this warning? It's an unused import to my understanding.

[k-rus]

I have two comments with questions after the latest change, which I like as it simplifies the code, and after my review on #2038. They might lead to changes or just replies. Re-request my review when they were addressed or replied.

[k-rus]

Is it possible to assert or test this assumption that the additional conditions on the column are not present in the row filter for a clustering column? Does it make sense to test/assert?

[adelapena]

The assumption in toCQLString methods is that the commands come from well-formed and validated CQL commands, I don't think we want the extra overload of validating/asserting on every consumer. Even more when in this case it's tricky that the IN restrictions of the clustering key are duplicated into the row filter as a serialized value, and we don't probably want to deserialize for an assertion. Can you think of any specific query that might not satisfy the behaviour described in the comment?

[k-rus]

I think I should be more clear about what I mean about the assumption. The assumption is that rowFilter contains only the duplicated conditions on clustering columns and, thus, removing all conditions on the clustering column is fine. If it will be changed in future that other conditions on the clustering column will be included by the validator, then how to make sure to update printing those conditions in this code? Is there a way to implement a test, which will have an extra condition on clustering column, which is currently not included into rowFilter?
Or do I misunderstand your comment about rowFilter content in the case of clustering column?

[adelapena]

I have added included some defensive checks in testToCQLString about the CQL semantics regarding clustering columns, so if CQL somehow gets to change that way we detect that toCQLString methods might need to change too. Not really sure we should really do this kind of defensive downstream checks, but glad to have them if it help us moving forward.

[adelapena]

@k-rus I think I have addressed the two new comments in the second round of review of this PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cassci-bot]

✔️ Build ds-cassandra-pr-gate/PR-2083 approved by Butler

---

Approved by Butler
See build details here