bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

cql3/prepare_context: fix generating pk_indexes for duplicate named bind variables

Browse Source 
When presented with queries that use the same named bind variables twice,
like this one:
```cql
SELECT p FROM table WHERE p = :x AND c = :x
```

Scylla generated empty partition_key_bind_indexes (pk_indexes).
pk_indexes tell the driver which bind variables it should use to calculate the partition
token for a query. Without it, the driver is unable to determine the token and it will
send the query to a random node.

Scylla should generate pk_indexes which tell the driver that it can use bind variable
with bind_index = 0 to calculate the partition token for a query.

The problem was that _target_columns keep only a single target_column for each bind variable.
In the example above :x is compared with both p and c, but _target_columns would contain
only one of them, and Scylla wasn't able to tell that this bind variable is compared with
a partition key column.

To fix it, let's replace _target_columns with _targets. _targets keeps all comparisons
between bind variables and other expressions, so none of them will be forgotten/overwritten.

Fixes: #15374

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
This commit is contained in:
Jan Ciolek
2023-09-22 18:23:41 +02:00
parent a993ae31f8
commit 3cff10f756
2 changed files with 11 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
cql3/prepare_context.cc
View File

@@ -30,11 +30,11 @@ std::vector<uint16_t> prepare_context::get_partition_key_bind_indexes(const sche
auto count = schema.partition_key_columns().size();
std::vector<uint16_t> partition_key_positions(count, uint16_t(0));
std::vector<bool> set(count, false);
for (size_t i = 0; i < _target_columns.size(); i++) {
auto& target_column = _target_columns[i];
const auto* cdef = target_column ? schema.get_column_definition(target_column->name->name()) : nullptr;
for (auto&& [bind_index, target_spec] : _targets) {
const auto* cdef = target_spec ? schema.get_column_definition(target_spec->name->name()) : nullptr;
if (cdef && cdef->is_partition_key()) {
partition_key_positions[cdef->position()] = i;
partition_key_positions[cdef->position()] = bind_index;
set[cdef->position()] = true;
}
}
@@ -56,7 +56,7 @@ void prepare_context::add_variable_specification(int32_t bind_index, lw_shared_p
*name, _variable_specs[bind_index]->type->as_cql3_type(), spec->name));
}
}
_target_columns[bind_index] = spec;
_targets.emplace_back(bind_index, spec);
// Use the user name, if there is one
if (name) {
spec = make_lw_shared<column_specification>(spec->ks_name, spec->cf_name, name, spec->type);
@@ -67,11 +67,11 @@ void prepare_context::add_variable_specification(int32_t bind_index, lw_shared_p
void prepare_context::set_bound_variables(const std::vector<shared_ptr<column_identifier>>& bind_variable_names) {
_variable_names = bind_variable_names;
_variable_specs.clear();
_target_columns.clear();
_targets.clear();
const size_t bn_size = bind_variable_names.size();
_variable_specs.resize(bn_size);
_target_columns.resize(bn_size);
_targets.resize(bn_size);
}
void prepare_context::clear_pk_function_calls_cache() {

7
cql3/prepare_context.hh
View File

@@ -38,11 +38,12 @@ private:
std::vector<shared_ptr<column_identifier>> _variable_names;
// Keeps column_specification for every bind_index. column_specification describes the name and type of this variable.
// It's different from _target_columns because it describes the name of this variable, not the target column.
std::vector<lw_shared_ptr<column_specification>> _variable_specs;
// The column to which bind variable with a given bind_index will be assigned to.
std::vector<lw_shared_ptr<column_specification>> _target_columns;
// For every expression like (<target> = <bind variable>), there's a pair of (bind_index, target column_specification) in _targets.
// Collecting all equalities of bind variables allows to determine which of the variables set the value of partition key columns.
// The driver needs this information in order to compute the partition token and send the request to the right node.
std::vector<std::pair<std::size_t, lw_shared_ptr<column_specification>>> _targets;
// A list of pointers to prepared `function_call` cache ids, that
// participate in partition key ranges computation within an LWT statement.