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

cql3: Don't look for indexed column in CK prefix

Browse Source 
When creating an index-table query, we form its clustering-key
restrictions by picking the right restrictions from the WHERE clause.
But we skip the indexed column, which isn't in the index-table clutering
key.  This is, however, both incorrect and unnecessary:

It is incorrect because we compare the column IDs from different schemas
(indexed table vs. base table).  We should instead be comparing column
names.

It is unnecessary because this code is only executed when the whole
partition key plus a clustering prefix is specified in the WHERE clause.
In such cases, the index cannot possibly be on a member of the
clustering prefix, as such a query would be satisfied out of the base
table.  Therefore, it is redundant to check for the indexed table among
the CK prefix elements.

A careful reader will note that this check was first introduced to fix
the issue #7888 in commit 0bd201d.  But it now seems to me that that fix
was misguided.  The root problem was the old code miscalculating the
clustering prefix by including too many columns in it; it should have
stopped before reaching the indexed column.  The new code, introduced by
commit 845e36e76, calculates the clustering prefix correctly, never
reaching the indexed column.

(Details, for the curious: the old code invoked
 clustering_key_restrictions::prefix_size(), which is buggy -- it
 doesn't check the restriction operator.  It will, for instance,
 calculate the prefix of `c1=0 AND c2 CONTAINS 0 AND c3=0` as 3, because
 it restricts c1, c2, and c3.  But the correct prefix is clearly 1,
 because c2 is not restricted by equality.)

Tests: unit (dev, debug)

Signed-off-by: Dejan Mircevski <dejan@scylladb.com>

Closes #8993
This commit is contained in:
Dejan Mircevski
2021-07-08 09:39:27 -04:00
committed by Nadav Har'El
parent 9059514335
commit 7119730f2d
1 changed files with 0 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cql3/restrictions/statement_restrictions.cc
View File

@@ -1353,7 +1353,6 @@ void statement_restrictions::prepare_indexed(const schema& idx_tbl_schema, bool
const auto pos = _schema->position(*col) + 1;
(*_idx_tbl_ck_prefix)[pos] = replace_column_def(e, &idx_tbl_schema.clustering_column_at(pos));
}
const column_definition& indexed_column = idx_tbl_schema.column_at(column_kind::partition_key, 0);
for (const auto& e : _clustering_prefix_restrictions) {
if (find_atom(_clustering_prefix_restrictions[0], expr::is_multi_column)) {
// TODO: We could handle single-element tuples, eg. `(c)>=(123)`.
@@ -1364,9 +1363,6 @@ void statement_restrictions::prepare_indexed(const schema& idx_tbl_schema, bool
break;
}
const auto col = std::get<column_value>(*any_binop->lhs).col;
if (*col == indexed_column) {
continue;
}
_idx_tbl_ck_prefix->push_back(replace_column_def(e, idx_tbl_schema.get_column_definition(col->name())));
}
auto token_column = &idx_tbl_schema.clustering_column_at(0);