Skip to content

Commit

Permalink
[YCQL] #3281 Fixed the Issue in Upgrade for SELECT from Indexed Table
Browse files Browse the repository at this point in the history
Summary:
After upgrade server of 2.0 and later versions did not process the existing pre-2.0.5 indexes in the database correctly.

- INDEXes that were created before 2.0 releases does not contain column_names.
- Newer server uses column names to determine if an expression is covered by the index.

The fix
- For existing INDEXes, we continue to use column IDs when checking for covering.
- Also, when checking for covering, to be safe, the predicate `column_name.empty()` is used. The function "string::find(str, substr)" will return TRUE if "substr" is empty.

Test Plan:
Test manually.
```
CREATE TABLE test.bbb (
    k int PRIMARY KEY,
    time int,
    height bigint
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};
CREATE INDEX bbb_height ON test.bbb (height, k)
    WITH CLUSTERING ORDER BY (k ASC)
    AND transactions = {'enabled': 'false', 'consistency_level': 'user_enforced'};

cqlsh:test> EXPLAIN SELECT * FROM bbb WHERE height = 1;

 QUERY PLAN
----------------------------------------------
 Index Scan using test.bbb_height on test.bbb
   Key Conditions: (height = 1)

cqlsh:test> describe bbb1;

CREATE TABLE test.bbb1 (
    k int PRIMARY KEY,
    time int,
    height bigint
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'false'};
CREATE INDEX bbb_height1 ON test.bbb1 (height, k) INCLUDE (time)
    WITH CLUSTERING ORDER BY (k ASC)
    AND transactions = {'enabled': 'false', 'consistency_level': 'user_enforced'};

cqlsh:test> EXPLAIN SELECT * FROM bbb1 WHERE height = 1;

 QUERY PLAN
-----------------------------------------------------
 Index Only Scan using test.bbb_height1 on test.bbb1
   Key Conditions: (height = 1)

cqlsh:test> describe bbb2;

CREATE TABLE test.bbb2 (
    k int PRIMARY KEY,
    time int,
    height bigint
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'true'};
CREATE INDEX bbb_height2 ON test.bbb2 (height, k)
    WITH CLUSTERING ORDER BY (k ASC)
    AND transactions = {'enabled': 'true'};

cqlsh:test> EXPLAIN SELECT * FROM bbb2 WHERE height = 1;

 QUERY PLAN
------------------------------------------------
 Index Scan using test.bbb_height2 on test.bbb2
   Key Conditions: (height = 1)

cqlsh:test> describe bbb3;

CREATE TABLE test.bbb3 (
    k int PRIMARY KEY,
    time int,
    height bigint
) WITH default_time_to_live = 0
    AND transactions = {'enabled': 'true'};
CREATE INDEX bbb_height3 ON test.bbb3 (height, k) INCLUDE (time)
    WITH CLUSTERING ORDER BY (k ASC)
    AND transactions = {'enabled': 'true'};

cqlsh:test> EXPLAIN SELECT * FROM bbb3 WHERE height = 1;

 QUERY PLAN
-----------------------------------------------------
 Index Only Scan using test.bbb_height3 on test.bbb3
   Key Conditions: (height = 1)
```

Reviewers: amitanand, mihnea

Reviewed By: mihnea

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D7768
  • Loading branch information
nocaway committed Jan 8, 2020
1 parent c02e1bf commit c25e3d9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/yb/common/index.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ int32_t IndexInfo::IsExprCovered(const string& expr_name) const {
// CREATE INDEX jindex on tab(j->'b'->>'a');
int32_t idx = 0;
for (const auto &col : columns_) {
if (expr_name.find(col.column_name) != expr_name.npos) {
if (!col.column_name.empty() && expr_name.find(col.column_name) != expr_name.npos) {
return idx;
}
idx++;
Expand All @@ -161,7 +161,7 @@ int32_t IndexInfo::IsExprCovered(const string& expr_name) const {
int32_t IndexInfo::FindKeyIndex(const string& key_expr_name) const {
for (int32_t idx = 0; idx < key_column_count(); idx++) {
const auto& col = columns_[idx];
if (key_expr_name.find(col.column_name) != key_expr_name.npos) {
if (!col.column_name.empty() && key_expr_name.find(col.column_name) != key_expr_name.npos) {
// Return the found key column that is referenced by the expression.
return idx;
}
Expand Down
14 changes: 14 additions & 0 deletions src/yb/yql/cql/ql/ptree/pt_select.cc
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,20 @@ CHECKED_STATUS PTSelectStmt::AnalyzeIndexes(SemContext *sem_context) {

// Return whether the index covers the read fully.
bool PTSelectStmt::CoversFully(const IndexInfo& index_info) const {
// INDEXes that were created before v2.0 are defined by column IDs instead of mangled_names.
// - Returns TRUE if a list of column refs of a statement is a subset of INDEX columns.
// - Use ColumnID to check if a column in a query is covered by the index.
// - The list "column_refs_" contains IDs of all columns that are referred to by SELECT.
// - The list "IndexInfo::columns_" contains the IDs of all columns in the INDEX.
if (!index_info.use_mangled_column_name()) {
for (const int32 table_col_id : column_refs_) {
if (!index_info.IsColumnCovered(ColumnId(table_col_id))) {
return false;
}
}
return true;
}

// Check all ColumnRef in selected list.
for (const PTExpr *expr : covering_exprs_) {
// If this expression does not have column reference, it is considered "coverred".
Expand Down

0 comments on commit c25e3d9

Please sign in to comment.