From a28dd56440c58046821edab782468ef40020681e Mon Sep 17 00:00:00 2001 From: Dawid Medrek Date: Thu, 29 Aug 2024 13:56:32 +0200 Subject: [PATCH 1/4] cqlsh.py: Fix indentation --- bin/cqlsh.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/cqlsh.py b/bin/cqlsh.py index 23afd5b..94e0224 100755 --- a/bin/cqlsh.py +++ b/bin/cqlsh.py @@ -488,7 +488,7 @@ def __init__(self, hostname, port, color=False, if os.path.exists(self.hostname) and stat.S_ISSOCK(os.stat(self.hostname).st_mode): kwargs['contact_points'] = (UnixSocketEndPoint(self.hostname),) self.profiles[EXEC_PROFILE_DEFAULT].load_balancing_policy = WhiteListRoundRobinPolicy([UnixSocketEndPoint(self.hostname)]) - else: + else: kwargs['contact_points'] = (self.hostname,) self.profiles[EXEC_PROFILE_DEFAULT].load_balancing_policy = WhiteListRoundRobinPolicy([self.hostname]) kwargs['port'] = self.port @@ -1669,7 +1669,7 @@ def _do_describe(self, parsed, force_client_side_describe): self.describe_element(result) except cassandra.protocol.SyntaxException: - # Server doesn't support DESCRIBE query, retry with + # Server doesn't support DESCRIBE query, retry with # client-side DESCRIBE implementation self._do_describe(parsed, force_client_side_describe=True) except CQL_ERRORS as err: @@ -2520,7 +2520,7 @@ def read_options(cmdlineargs, environment): parser.error("Cannot use --cloudconf with hostname or port") if options.ssl: parser.error("Cannot use --cloudconf with --ssl. Cloud connection encryption parameters are provided by cloud config bundle.") - + hostname = option_with_default(configs.get, 'connection', 'hostname', DEFAULT_HOST) port = option_with_default(configs.get, 'connection', 'port', DEFAULT_PORT) From 411b662a9bc0211d9e48d7b403ba38389c9b9d8b Mon Sep 17 00:00:00 2001 From: Dawid Medrek Date: Thu, 29 Aug 2024 14:18:15 +0200 Subject: [PATCH 2/4] cqlsh.py: Send DESCRIBE statement to server before parsing Attempting to parse a DESCRIBE statement before performing it forces us to update cqlsh whenever a new element of the schema that could be described is introduced. We should try to rely on the server and only in the case of a failure (when the version of the server is old and it doesn't recognize the query) should we try to parse it and perform locally. Trying to parse it first may lead to rejecting DESCRIBE statements that would otherwise be valid -- if the grammar used by cqlsh hasn't caught up with Scylla yet. --- bin/cqlsh.py | 92 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 37 deletions(-) diff --git a/bin/cqlsh.py b/bin/cqlsh.py index 94e0224..9b38d34 100755 --- a/bin/cqlsh.py +++ b/bin/cqlsh.py @@ -1008,6 +1008,21 @@ def handle_statement(self, tokens, srcstr): cmdword = tokens[0][1] if cmdword == '?': cmdword = 'help' + + cmdword_lower = cmdword.lower() + + # Describe statements get special treatment: we first want to + # send the request to the server and only when it fails will + # we attempt to perform it locally. That's why we don't want + # to follow the logic below that starts with parsing. + # + # The reason for that is changes in Scylla may need to be reflected + # in the grammar used in cqlsh. We want Scylla to be "independent" + # in that regard, so unless necessary, we don't want to do the parsing + # here. + if cmdword_lower == 'describe' or cmdword_lower == 'desc': + return self.perform_describe(cmdword, tokens, srcstr) + custom_handler = getattr(self, 'do_' + cmdword.lower(), None) if custom_handler: parsed = cqlruleset.cql_whole_parse_tokens(tokens, srcstr=srcstr, @@ -1497,8 +1512,8 @@ def describe_schema_client(self, include_system=False): self.print_recreate_keyspace(k, sys.stdout) print('') - def do_describe(self, parsed): - + # Precondition: the first token in `srcstr.lower()` is either `describe` or `desc`. + def perform_describe(self, cmdword, tokens, srcstr): """ DESCRIBE [cqlsh only] @@ -1589,10 +1604,8 @@ def do_describe(self, parsed): where object can be either a keyspace or a table or an index or a materialized view (in this order). """ - self._do_describe(parsed, force_client_side_describe=False) - def _do_describe(self, parsed, force_client_side_describe): - if force_client_side_describe: + def perform_describe_locally(parsed): what = parsed.matched[1][1].lower() if what == 'functions': self.describe_functions_client(self.current_keyspace) @@ -1650,40 +1663,45 @@ def _do_describe(self, parsed, force_client_side_describe): if not name: name = self.cql_unprotect_name(parsed.get_binding('mvname', None)) self.describe_object_client(ks, name) - else: - stmt = SimpleStatement(parsed.extract_orig(), consistency_level=cassandra.ConsistencyLevel.LOCAL_ONE, - fetch_size=self.page_size if self.use_paging else None) - future = self.session.execute_async(stmt) - try: - result = future.result() - - what = parsed.matched[1][1].lower() - - if what in ('columnfamilies', 'tables', 'types', 'functions', 'aggregates'): - self.describe_list(result) - elif what == 'keyspaces': - self.describe_keyspaces(result) - elif what == 'cluster': - self.describe_cluster(result) - elif what: - self.describe_element(result) - - except cassandra.protocol.SyntaxException: - # Server doesn't support DESCRIBE query, retry with - # client-side DESCRIBE implementation - self._do_describe(parsed, force_client_side_describe=True) - except CQL_ERRORS as err: - err_msg = err.message if hasattr(err, 'message') else str(err) - self.printerr(err_msg.partition("message=")[2].strip('"')) - except Exception: - import traceback - self.printerr(traceback.format_exc()) - if future: - if future.warnings: - self.print_warnings(future.warnings) + stmt = SimpleStatement(srcstr, consistency_level=cassandra.ConsistencyLevel.LOCAL_ONE, + fetch_size=self.page_size if self.use_paging else None) + future = self.session.execute_async(stmt) + try: + result = future.result() + + # The second token in the statement indicates which + # kind of DESCRIBE we're performing. + what = srcstr.split()[1].lower().rstrip(';') + + if what in ('columnfamilies', 'tables', 'types', 'functions', 'aggregates'): + self.describe_list(result) + elif what == 'keyspaces': + self.describe_keyspaces(result) + elif what == 'cluster': + self.describe_cluster(result) + elif what: + self.describe_element(result) + + except cassandra.protocol.SyntaxException: + # Server doesn't support DESCRIBE query, retry with + # client-side DESCRIBE implementation + parsed = cqlruleset.cql_whole_parse_tokens(tokens, srcstr=srcstr, + startsymbol='cqlshCommand') + if parsed and not parsed.remainder: + return perform_describe_locally(parsed) + else: + return self.handle_parse_error(cmdword, tokens, parsed, srcstr) + except CQL_ERRORS as err: + err_msg = err.message if hasattr(err, 'message') else str(err) + self.printerr(err_msg.partition("message=")[2].strip('"')) + except Exception: + import traceback + self.printerr(traceback.format_exc()) - do_desc = do_describe + if future: + if future.warnings: + self.print_warnings(future.warnings) def describe_keyspaces(self, rows): """ From 747dc895d4a417d17841fc54331e488042fa5553 Mon Sep 17 00:00:00 2001 From: Dawid Medrek Date: Thu, 29 Aug 2024 16:07:17 +0200 Subject: [PATCH 3/4] cqlshlib/test: Use assertEqual instead of assertEquals The latter method is not declared, so use the former. --- pylib/cqlshlib/test/test_cqlsh_output.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylib/cqlshlib/test/test_cqlsh_output.py b/pylib/cqlshlib/test/test_cqlsh_output.py index eb8929a..9c85b83 100644 --- a/pylib/cqlshlib/test/test_cqlsh_output.py +++ b/pylib/cqlshlib/test/test_cqlsh_output.py @@ -1061,14 +1061,14 @@ def test_scylla_tags(self): AND min_index_interval = 128 AND read_repair_chance = 0.0 AND speculative_retry = '99.0PERCENTILE'; - + cdc = {{'delta': 'full', 'enabled': 'true', 'postimage': 'false', 'preimage': 'false', 'ttl': '86400'}} """) with testrun_cqlsh(tty=True, env=self.default_env) as c: output = c.cmd_and_response(f"CREATE TABLE {qks}.ccc (pkey int, PRIMARY KEY(pkey)) WITH cdc = {{'enabled': true}};") - self.assertEquals(output.strip(), "") + self.assertEqual(output.strip(), "") output = c.cmd_and_response('describe table {}.ccc'.format(qks)) lines = _normalize_response(dedent(output)) expected_lines = _normalize_response(expected) From c57ea6ef7e836330c248e9131ef219991038aabe Mon Sep 17 00:00:00 2001 From: Dawid Medrek Date: Thu, 29 Aug 2024 16:31:55 +0200 Subject: [PATCH 4/4] cqlshlib/test: Add test_formatting.py We add two small tests to verify that splitting DESCRIBE statements work correctly. We do it by hand now and we need to make sure nothing has broken. --- pylib/cqlshlib/test/test_formatting.py | 69 ++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 pylib/cqlshlib/test/test_formatting.py diff --git a/pylib/cqlshlib/test/test_formatting.py b/pylib/cqlshlib/test/test_formatting.py new file mode 100644 index 0000000..9801daa --- /dev/null +++ b/pylib/cqlshlib/test/test_formatting.py @@ -0,0 +1,69 @@ +# coding=utf-8 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import string + +from .basecase import BaseTestCase +from .cassconnect import (get_cassandra_connection, create_keyspace, remove_db, testrun_cqlsh) + + +class TestFormatting(BaseTestCase): + + @classmethod + def setUpClass(cls): + s = get_cassandra_connection().connect() + s.default_timeout = 60.0 + create_keyspace(s) + s.execute('CREATE TABLE t (k int PRIMARY KEY, v text)') + + env = os.environ.copy() + env['LC_CTYPE'] = 'UTF-8' + cls.default_env = env + + @classmethod + def tearDownClass(cls): + remove_db() + + def test_multiple_semicolons_in_describe(self): + with testrun_cqlsh(tty=True, env=self.default_env) as c: + v1 = 'type_name' + v2 = 'value_name' + _ = c.cmd_and_response(f'CREATE TYPE "{v1}" ( "{v2}" int );') + output = c.cmd_and_response('DESC TYPES;;;') + self.assertIn(v1, output) + output = c.cmd_and_response(f'DESC TYPE "{v1}";;;') + self.assertIn(v2, output) + + def test_spaces_in_describe(self): + with testrun_cqlsh(tty=True, env=self.default_env) as c: + v1 = 'type_name' + v2 = 'value_name' + _ = c.cmd_and_response(f'CREATE TYPE "{v1}" ( "{v2}" int );') + + def verify_output(prefix_str: str, infix_str: str, suffix_str: str) -> None: + output = c.cmd_and_response(f'DESC{prefix_str}TYPES{suffix_str};') + self.assertIn(v1, output) + output = c.cmd_and_response(f'DESC{prefix_str}TYPE{infix_str}"{v1}"{suffix_str};') + self.assertIn(v2, output) + + # cqlsh doesn't work well with whitespace characters other than spaces apparently. + spaces = [' ', ' ', ' '] + for prefix in spaces: + for infix in spaces: + for suffix in [*spaces, '']: + verify_output(prefix, infix, suffix)