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

column_computation.hh, schema.cc: compute_value interface refactor

Browse Source 
The compute_value function of column_computation has had previously the
following signature:
    virtual bytes_opt compute_value(const schema& schema, const partition_key& key, const clustering_row& row) const override;

This is superfluous, since never in the history of Scylla, the last
parameter (row) was used in any implentation, and never did it happen
that it returned bytes_opt. The absurdity of this interface can be seen
especially when looking at call sites like following, where dummy empty
row was created:
```
    token_column.get_computation().compute_value(
            *_schema, pkv_linearized, clustering_row(clustering_key_prefix::make_empty()));
```
This commit is contained in:
Michał Radwański
2022-05-17 15:48:27 +02:00
committed by Nadav Har'El
parent 166afd46b5
commit 2babee2cdc
5 changed files with 16 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
column_computation.hh
View File

@@ -22,7 +22,7 @@ using column_computation_ptr = std::unique_ptr<column_computation>;
* Computed columns description is also available at docs/dev/system_schema_keyspace.md. They hold values
* not provided directly by the user, but rather computed: from other column values and possibly other sources.
* This class is able to serialize/deserialize column computations and perform the computation itself,
* based on given schema, partition key and clustering row. Responsibility for providing enough data
* based on given schema, and partition key. Responsibility for providing enough data
* in the clustering row in order for computation to succeed belongs to the caller. In particular,
* generating a value might involve performing a read-before-write if the computation is performed
* on more values than are present in the update request.
@@ -36,7 +36,7 @@ public:
virtual column_computation_ptr clone() const = 0;
virtual bytes serialize() const = 0;
virtual bytes_opt compute_value(const schema& schema, const partition_key& key, const clustering_row& row) const = 0;
virtual bytes compute_value(const schema& schema, const partition_key& key) const = 0;
};
/*
@@ -54,7 +54,7 @@ public:
return std::make_unique<legacy_token_column_computation>(*this);
}
virtual bytes serialize() const override;
virtual bytes_opt compute_value(const schema& schema, const partition_key& key, const clustering_row& row) const override;
virtual bytes compute_value(const schema& schema, const partition_key& key) const override;
};
@@ -75,5 +75,5 @@ public:
return std::make_unique<token_column_computation>(*this);
}
virtual bytes serialize() const override;
virtual bytes_opt compute_value(const schema& schema, const partition_key& key, const clustering_row& row) const override;
virtual bytes compute_value(const schema& schema, const partition_key& key) const override;
};

11
cql3/restrictions/statement_restrictions.cc
View File

@@ -1879,17 +1879,12 @@ std::vector<query::clustering_range> statement_restrictions::get_global_index_cl
std::transform(pk_value.cbegin(), pk_value.cend(), pkv_linearized.begin(),
[] (const managed_bytes& mb) { return to_bytes(mb); });
auto& token_column = idx_tbl_schema.clustering_column_at(0);
bytes_opt token_bytes = token_column.get_computation().compute_value(
*_schema, pkv_linearized, clustering_row(clustering_key_prefix::make_empty()));
if (!token_bytes) {
on_internal_error(rlogger,
format("null value for token column in indexing table {}",
token_column.name_as_text()));
}
bytes token_bytes = token_column.get_computation().compute_value(*_schema, pkv_linearized);
// WARNING: We must not yield to another fiber from here until the function's end, lest this RHS be
// overwritten.
const_cast<expr::expression&>(expr::as<binary_operator>((*_idx_tbl_ck_prefix)[0]).rhs) =
expr::constant(raw_value::make_value(*token_bytes), token_column.type);
expr::constant(raw_value::make_value(token_bytes), token_column.type);
// Multi column restrictions are not added to _idx_tbl_ck_prefix, they are handled later by filtering.
return get_single_column_clustering_bounds(options, idx_tbl_schema, *_idx_tbl_ck_prefix);

11
cql3/statements/select_statement.cc
View File

@@ -965,18 +965,15 @@ static void append_base_key_to_index_ck(std::vector<managed_bytes_view>& explode
bytes indexed_table_select_statement::compute_idx_token(const partition_key& key) const {
const column_definition& cdef = *_view_schema->clustering_key_columns().begin();
clustering_row empty_row(clustering_key_prefix::make_empty());
bytes_opt computed_value;
bytes computed_value;
if (!cdef.is_computed()) {
// FIXME(pgrabowski): this legacy code is here for backward compatibility and should be removed
// once "computed_columns feature" is supported by every node
computed_value = legacy_token_column_computation().compute_value(*_schema, key, empty_row);
computed_value = legacy_token_column_computation().compute_value(*_schema, key);
} else {
computed_value = cdef.get_computation().compute_value(*_schema, key, empty_row);
computed_value = cdef.get_computation().compute_value(*_schema, key);
}
if (!computed_value) {
throw std::logic_error(format("No value computed for idx_token column {}", cdef.name()));
}
return *computed_value;
return computed_value;
}
lw_shared_ptr<const service::pager::paging_state> indexed_table_select_statement::generate_view_paging_state_from_base_query_results(lw_shared_ptr<const service::pager::paging_state> paging_state,

7
db/view/view.cc
View File

@@ -495,12 +495,9 @@ deletable_row& view_updates::get_view_row(const partition_key& base_key, const c
if (!service::get_local_storage_proxy().local_db().find_column_family(_base->id()).get_index_manager().is_index(*_view)) {
throw std::logic_error(format("Column {} doesn't exist in base and this view is not backing a secondary index", cdef.name_as_text()));
}
computed_value = legacy_token_column_computation().compute_value(*_base, base_key, update);
computed_value = legacy_token_column_computation().compute_value(*_base, base_key);
} else {
computed_value = cdef.get_computation().compute_value(*_base, base_key, update);
}
if (!computed_value) {
throw std::logic_error(format("No value computed for primary key column {}", cdef.name()));
computed_value = cdef.get_computation().compute_value(*_base, base_key);
}
return managed_bytes_view(linearized_values.emplace_back(*computed_value));
}

6
schema.cc
View File

@@ -1706,8 +1706,8 @@ bytes legacy_token_column_computation::serialize() const {
return to_bytes(rjson::print(serialized));
}
bytes_opt legacy_token_column_computation::compute_value(const schema& schema, const partition_key& key, const clustering_row& row) const {
return dht::get_token(schema, key).data();
bytes legacy_token_column_computation::compute_value(const schema& schema, const partition_key& key) const {
return {dht::get_token(schema, key).data()};
}
bytes token_column_computation::serialize() const {
@@ -1716,7 +1716,7 @@ bytes token_column_computation::serialize() const {
return to_bytes(rjson::print(serialized));
}
bytes_opt token_column_computation::compute_value(const schema& schema, const partition_key& key, const clustering_row& row) const {
bytes token_column_computation::compute_value(const schema& schema, const partition_key& key) const {
auto long_value = dht::token::to_int64(dht::get_token(schema, key));
return long_type->decompose(long_value);
}