Skip to content

Commit

Permalink
Merge 'cql3:statements:describe_statement: check pointer after castin…
Browse files Browse the repository at this point in the history
…g to UDF/UDA' from Michał Jadwiszczak

There was a bug in describe_statement. If executing `DESC FUNCTION  <uda name>` or ` DESC AGGREGATE <udf name>`, Scylla was crashing because the function was found (`functions::find()` searches both UDFs and UDAs) but the function was bad and the pointer wasn't checked after cast.

Added a test for this.

Fixes: #14360

Closes #14332

* github.com:scylladb/scylladb:
  cql-pytest:test_describe: add test for filtering UDF and UDA
  cql3:statements:describe_statement: check pointer to UDF/UDA
  • Loading branch information
nyh committed Jun 22, 2023
2 parents 23a60df + d3d9a15 commit 0a1283c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
14 changes: 13 additions & 1 deletion cql3/statements/describe_statement.cc
Expand Up @@ -179,7 +179,13 @@ future<std::vector<description>> function(replica::database& db, const sstring&

auto udfs = boost::copy_range<std::vector<shared_ptr<const keyspace_element>>>(fs | boost::adaptors::transformed([] (const auto& f) {
return dynamic_pointer_cast<const functions::user_function>(f.second);
}) | boost::adaptors::filtered([] (const auto& f) {
return f != nullptr;
}));
if (udfs.empty()) {
throw exceptions::invalid_request_exception(format("Function '{}' not found in keyspace '{}'", name, ks));
}

co_return co_await generate_descriptions(db, udfs, true);
}

Expand All @@ -192,13 +198,19 @@ future<std::vector<description>> functions(replica::database& db,const sstring&

future<std::vector<description>> aggregate(replica::database& db, const sstring& ks, const sstring& name) {
auto fs = functions::functions::find(functions::function_name(ks, name));
if(fs.empty()) {
if (fs.empty()) {
throw exceptions::invalid_request_exception(format("Aggregate '{}' not found in keyspace '{}'", name, ks));
}

auto udas = boost::copy_range<std::vector<shared_ptr<const keyspace_element>>>(fs | boost::adaptors::transformed([] (const auto& f) {
return dynamic_pointer_cast<const functions::user_aggregate>(f.second);
}) | boost::adaptors::filtered([] (const auto& f) {
return f != nullptr;
}));
if (udas.empty()) {
throw exceptions::invalid_request_exception(format("Aggregate '{}' not found in keyspace '{}'", name, ks));
}

co_return co_await generate_descriptions(db, udas, true);
}

Expand Down
11 changes: 11 additions & 0 deletions test/cql-pytest/test_describe.py
Expand Up @@ -12,6 +12,7 @@
from pytest import fixture
from contextlib import contextmanager
from util import new_type, unique_name, new_test_table, new_test_keyspace, new_function, new_aggregate, new_cql
from cassandra.protocol import InvalidRequest

# (`element` refers to keyspace or keyspace's element(table, type, function, aggregate))
# There are 2 main types of tests:
Expand Down Expand Up @@ -459,6 +460,16 @@ def test_generic_desc(cql, random_seed):
assert generic_tbl == desc_tbl
assert generic_idx == desc_idx

# Test that 'DESC FUNCTION'/'DESC AGGREGATE' doesn't show UDA/UDF and doesn't crash Scylla
def test_desc_udf_uda(cql, test_keyspace):
with new_function(cql, test_keyspace, "(a int, b int) RETURNS NULL ON NULL INPUT RETURNS int LANGUAGE LUA AS 'return a+b'") as fn:
with new_aggregate(cql, test_keyspace, f"(int) SFUNC {fn} STYPE int") as aggr:

with pytest.raises(InvalidRequest):
cql.execute(f"DESC FUNCTION {test_keyspace}.{aggr}")
with pytest.raises(InvalidRequest):
cql.execute(f"DESC AGGREGATE {test_keyspace}.{fn}")

# -----------------------------------------------------------------------------
# Following tests `test_*_quoting` check if names inside elements' descriptions are quoted if needed.
# The tests don't check if create statements are correct, but only assert if the name inside is quoted.
Expand Down

0 comments on commit 0a1283c

Please sign in to comment.