execute_internal() duplicates several code paths, especially in
the select path, for no good reason. It boils down to timeout and
consistency level selection which can be done based on
client_state::is_internal().
This patchset eliminated the duplication and execute_internal(),
simplifying the code.
* github.com:avikivity/scylla cql-no-execute_internal/v2:
cql: schema_altering_statement: make execute() and execute_internal()
equivalent
cql: select_statement: make execute() and execute_internal()
equivalent
cql: query_processor: don't call cql_statement::execute_internal() any
more
cql: cql_statement: remove execute_internal()
As each test completes, report it. This prevents a long-running
test in the beginning of the list from stalling output.
Message-Id: <20180526173517.23078-1-avi@scylladb.com>
"
This series introduces frozen_mutation_fragment which can be used to
send mutation_fragments over the wire to a remote node. The main
intended user is going to be the new streaming implementation.
The first part of the series fixes some IDL issues related to empty
structures and variant being the first member of a structure. Both these
problems make the generated code fail to build and they do not, in any
way, affect the existing on-wire protocol.
Logic responsible for freezing and unfreezing of mutation_fragments is
heavily based on the existing code for freezing mutations and shares the
same drawbacks (for example, unnecessary copy during unfreezing). These
preexisting performance problems can be fixed incrementally.
Another performance problem (which affects frozen_mutations as well, but
to a lesser extent) is that since the batching is done at a different
layer each frozen mutation fragment is a separate bytes_ostream object
owning at least one memory buffer. If the mutation fragments are small
this will cause an excessive number of allocations. This could be solved
either by freezing fragments in batches (though it goes against the RPC
layer doing its own batching) or using bytes_ostream or an equivalent
object with a buffer allocation policy more suitable for such use cases.
This also is something that probably could be an incremental fix.
Tests: unit (release)
"
* tag 'frozen_mutation_fragment/v1-rebased' of https://github.com/pdziepak/scylla:
idl: add idl description of frozen_mutation_fragments
tests: add test for frozen_mutation_fragments
frozen_mutation: introduce frozen_mutation_fragment
tests/idl: test variant being the first member of a structure
idl: create variant state in root node
tests/idl: test serialising and deserialising empty structures
idl-compiler: avoid unused variable in empty struct deserialisers
tests/mutation_reader: disambiguate freeze() overload
All cql_statement::execute_internal() overrides now either throw or
call execute(). Since we shouldn't be calling the throwing overrides
internally, we can safely call execute() instead. This allows us to
get rid of execute_internal().
execute_internal(), for some code paths, differs from execute by the
following:
1. it uses CL_ONE unconditionally
2. it has no query timeout
3. it doesn't use execution stages
for other code paths, it just calls execute.
As preparation for getting rid of execute_internal(), unify the two
code paths.
Commit 4859b759b9 caused the consistency level and timeouts
to be provided by the caller, so using the caller provided parameters
instead of overriding them does not change behavior.
"
This patchset makes all users of query_processor specify their timeouts
explicitly, in preparation for the removal of
cql_statement::execute_internal() (whose main function was to override
timeouts).
"
* tag 'cql-explicit-timeouts/v1' of https://github.com/avikivity/scylla:
query_processor: require clients to specify timeout configuration
query_processor: un-default consistency level in make_internal_options
"
Firstly, this patchset removes the is_fixed_length() function of
abstract_type in favour of value_length_if_fixed().
Secondly, it fixed the byte_type to be compatible with Cassandra which
erroneously treats it as a variable-length data type.
Lastly, it adds a unit test covering all non-composite CQL data types
for writing.
Tests: unit {release}
"
* 'projects/sstables-30/different-data-types/v1' of https://github.com/argenet/scylla:
tests: Add a unit test for writing different data types to SSTables 3.x format.
types: Treat byte_type as a variable-length type for compatibility reasons.
types: Remove is_value_fixed() and use value_length_if_fixed() instead.
Although values of the byte_type that corresponds to CQL TINYINT type
always occupy only a single byte, Cassandra treats this it as a
variable-length type for SSTables 3.0 reading and writing.
While it is clearly a mistake at Cassandra side, we have to stay
compatible.
Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
This patch introduces IDL definition as well as serialisers and
deserialisers for freezing mutation_fragment so that they can be
transferred between nodes in a cluster.
Each non-final IDL object is preceeded by a frame containing its size.
In case of boost::variant there is a frame for the variant itself, an
integer determining the active alternative of the variant and a frame of
that active alternative.
However, if a variant was the first member of a writable stub object the
IDL would generate code that would not write the frame for the variant.
This is not a very severe issue since there are no such cases right now
as C++ type system would no allow such generated code to compile.
Deserialisers generated by IDL compiler first create a substream
covering the deserialised structure and then skip and read appropriate
members. If there are no members the substream will be unused and prompt
the compiler to emit a warning.
"
This patch series fixes#3405: secondary-index search only provided
correct results in certain cases, where entire partitions or contiguous
partition slices matched the query. When this was not the case, and
individual clustering rows match or do not match the query, the wrong
results were returned.
To fix this bug, we need to fix the two stages of secondary-index search:
1. In the first stage, we read from the index MV a list of row keys
(i.e., primary keys) matching the query. We can no longer remember
just the partition keys, and need to keep the list of full primary keys.
2. In the second stage, we have a list of rows (not partitions) and need
to read their selected contents to return to the user. Since CQL queries
do not have a syntax to select an arbitrary list of rows, we have to
add new code to do such a selection.
Because we provide an ad-hoc, inefficient, implementation for the row
selection described in stage 2, these patches leave two paths in the code:
The old path, efficiently selecting entire partitions, and the new path,
selecting individual rows. The old path is still used when it is applicable,
which is when a partition key column or the first clustering key column
is searched.
"
* 'si-fix-v4' of http://github.com/nyh/scylla:
secondary index: test multiple clustering column
secondary index: fix wrong results returned in certain cases
secondary index: method for fetching list of rows from base table
secondary index: method for fetching list of rows from index
select_statement.cc: refactor find_index_partition_ranges()
select_statement.cc: fix variable lifetime errors
This patch adds a test for secondary indexes on a table which has many
columns - two partition key column, two clustering key columns, and two
regular columns. We add a bunch of data in various rows and partitions,
index all columns and search on this data and verify the results.
This test exposed various bugs in secondary index search, including
issue #3405. After we fixed those bugs, the test now passes.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
The current secondary-index search code, in
indexed_table_select_statement::do_execute(), begins by fetching a list
of partitions, and then the content of these partitions from the base
table. However, in some cases, when the table has clustering columns and
not searching on the first one of them, doing this work in partition
granularity is wrong, and yields wrong results as demonstrated in
issue #3405.
So in this patch, we recognize the cases where we need to work in
clustering row granularity, and in those cases use the new functions
introduced in the previous patches - find_index_clustering_rows() and
the execute() variant taking a list of primary-keys of rows.
Fixes#3405.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
We add a new variant of select_statement::execute() which allows selecting
an arbitrary list of clustering rows. The existing execute() variant can't
do that - it can only take a list of *partitions*, and read the same
clustering rows from all of them.
The new select variant is not needed for regular CQL queries (which do
not have a syntax allowing reading a list of rows with arbitrary primary
keys), but we will need it for secondary index search, for solving
issue #3405.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
We already have a method find_index_partition_ranges(), to fetch a list
of partition keys from the secondary index. However, as we shall see in
the following patches (and see also issue #3405), getting a list of entire
partitions is not always enough - the secondary index actually holds a list
of primary keys, which includes clustering keys, and in some queries we
can't just ignore them.
So this patch provides a new method find_index_clustering_rows(), to
query the secondary index and get a list of matching clustering keys.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
The function find_index_partition_ranges() is used in secondary index
searches for fetching a list of matching partition. In a following patch,
we want to add a similar function for getting a list of *rows*. To avoid
duplicate code, in this patch we split parts of find_index_partition_ranges()
into two new functions:
1. get_index_schema() returns a pointer to the index view's schema.
2. read_posting_list() reads from this view the posting list (i.e., list
of keys) for the current searched value.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
do_with() provides code a *reference* to an object which will be kept
alive. It is a mistake to make a copy of this object or of parts of it,
because then the lifetime of this copy will have to be maintained as well.
In particular, it is a mistake to do do_with(..., [] (auto x) { ... }) -
note how "auto x" appears instead of the correct "auto& x". This causes
the object to be copied, and its lifetime not maintained.
This patch fixes several cases where this rule was broken in
select_statement.cc. I could not reproduce actual crashes caused by
these mistakes, but in theory they could have happened.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
"
This patchset implements reading row columns from SSTable 3 format data file.
Tests: units (release)
"
* 'haaawk/sstables3/read-columns-v4' of ssh://github.com/scylladb/seastar-dev: (21 commits)
Add test for reading column values of different types.
Support all fixed size column types from SSTable 3.x
Add abstract_type::value_length_if_fixed
Add test for simple table with value
flat_reader_assertions: Add produces_row taking column values
Implement reading rows and columns in data_consume_rows_context_m
Introduce column_flags_m
Add column_translation to data_consume_rows_context_m
Pass schema to data_consume_context
Add column_translation.hh
consumer_m: Add consume methods for consuming rows and columns
Extract make_atomic_cell from mp_row_consumer_k_l
Rename NON_STATIC_ROW_* states to ROW_BODY_*
Add liveness_info and use it in reading sstables
Add helper methods for parsing simple types.
Add unfiltered_flags_m::has_all_columns
data_consume_context: use make_unique instead of new
Pass serialization_header to data_consume_rows_context*
Use disk_string_vint_size for bytes_array_vint_size
Introduce disk_string_vint_size type
...
We need to specify --configfile on pdebuild too, otherwise we will
always fail to build .deb on newly created build environment.
Only reason why we still able to build .deb is we already copied
.pbuilderrc to home directory on existing build environment.
Fixes#3456
Signed-off-by: Takuya ASADA <syuu@scylladb.com>
Message-Id: <20180523204112.24669-1-syuu@scylladb.com>
It will be needed to obtain column_translation that will
be added to data_consume_context in the next patch.
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
New name describes the states in a better way as those states
will be used both for static and non-static rows.
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
