Add a test case which performs an LWT UPDATE, but the partition key
has 0 possible values, because it's supposed to be equal to two
different values.
Such queries used to cause problems in the past.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Our documentation states that writing an item with "USING TTL 0" means it
should never expire. This should be true even if the table has a default
TTL. But Scylla mistakenly handled "USING TTL 0" exactly like having no
USING TTL at all (i.e., it took the default TTL, instead of unlimited).
We had two xfailing tests demonstrating that Scylla's behavior in this
is different from Cassandra. Scylla's behavior in this case was also
undocumented.
By the way, Cassandra used to have the same bug (CASSANDRA-11207) but
it was fixed already in 2016 (Cassandra 3.6).
So in this patch we fix Scylla's "USING TTL 0" behavior to match the
documentation and Cassandra's behavior since 2016. One xfailing test
starts to pass and the second test passes this bug and fails on a
different one. This patch also adds a third test for "USING TTL ?"
with UNSET_VALUE - it behaves, on both Scylla and Cassandra, like a
missing "USING TTL".
The origin of this bug was that after parsing the statement, we saved
the USING TTL in an integer, and used 0 for the case of no USING TTL
given. This meant that we couldn't tell if we have USING TTL 0 or
no USING TTL at all. This patch uses an std::optional so we can tell
the case of a missing USING TTL from the case of USING TTL 0.
Fixes#6447
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13079
Add a test which performs an UPDATE and
tries to pass an UNSET_VALUE as a value
for the primary key.
There is also an LWT variant of this test
that tries to set an UNSET_VALUE
in the IF condition.
These two tests are analogous to
test_insert_update_where and
test_insert_update_where_lwt,
but use an UPDATE instead of INSERT.
It's useful to test UPDATE as well as INSERT.
When I was developing a fix for #13001
I initially added the condition for unset value
inside insert_statement, but this didn't handle
update statements. These two tests allowed me
to see that UPDATE still causes a crash.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Closes#13058
When Fedora 37 came out, we discovered that its "pytest" script started
to run Python with the "-s" option, which caused problems for packages
installed personally via pip. We fixed this by adding our own wrapper
script test/pytest.
But this bug (https://bugzilla.redhat.com/show_bug.cgi?id=2152171) was
already fixed in Fedora 37, and the new version already reached our
dbuild. So we no longer need this wrapper script. Let's remove it.
Fixes#12412
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13083
when comparing the disabled warnings specified by `configured.py` and the ones specified by `cmake/mode.common.cmake`, it turns out we are now able to enable more warning options. so let's enable them. the change was tested using Clang-17 and GCC-13.
there are many errors from GCC-13, like:
```
/home/kefu/dev/scylladb/db/view/view.hh:114:17: error: declaration of ‘column_kind db::view::clustering_or_static_row::column_kind() const’ changes meaning of ‘column_kind’ [-fpermissive]
114 | column_kind column_kind() const {
| ^~~~~~~~~~~
```
so the build with GCC failed.
and with this change, Clang-17 is able to build build the tree without warnings.
Closes#13096
* github.com:scylladb/scylladb:
build: enable more warnings
test: do not initialize plain number with {}
test: do not initialize a time_t with braces
The code for compare_endpoints originates at the dawn of time (bc034aeaec)
and is called on the fast path from storage_proxy via `sort_by_proximity`.
This series considerably reduces the function's footprint by:
1. carefully coding the many comparisons in the function so to reduce the number of conditional banches (apparently the compiler isn't doing a good enough job at optimizing it in this case)
2. avoid sstring copy in topology::get_{datacenter,rack}
Closes#12761
* github.com:scylladb/scylladb:
topology: optimize compare_endpoints
to_string: add print operators for std::{weak,partial}_ordering
utils: to_sstring: deinline std::strong_ordering print operator
move to_string.hh to utils/
test: network_topology: add test_topology_compare_endpoints
One test in test/cql-pytest/test_batch.py accidentally had the asyncio
marker, despite not using any async features. Remove it. The test still
runs fine.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13002
time_t is defined as a "Arithmetic type capable of representing times".
so we can just initialize it with 0 without braces. this change should
silence warning like:
```
test/boost/aggregate_fcts_test.cc:238:45: error: braces around scalar initializer [-Werror,-Wbraced-scalar-init]
auto tp = db_clock::from_time_t({ 0 }) + std::chrono::milliseconds(1);
^~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The sstable_compaction_test::simple_backlog_controller_test makes
sstables with empty dir argument. Eventually this means that sstables
happen in / directory [1], which's not nice.
As a side effect this also makes sstable::storage::prefix() returns
empty string which, in turn, confuses the code that tries to analyze the
prefix contents (refs: #13090)
[1] See, e.g. logs from https://jenkins.scylladb.com/job/releng/job/Scylla-CI/4757/consoleText
```
INFO 2023-03-06 21:23:04,536 [shard 0] compaction - [Compact ks.cf 51489760-bc54-11ed-a08c-7d3f1d77e2e4] Compacting [/la-1-big-Data.db:level=0:origin=]
```
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#13094
When the WASM UDFs were first introduced, the LANGUAGE required in
the CQL statements to use them was "xwasm", because the ABI for the
UDFs was still not specified and changes to it could be backwards
incompatible.
Now, the ABI is stabilized, but if backwards incompatible changes
are made in the future, we will add a new ABI version for them, so
the name "xwasm" is no longer needed and we can finally
change it to "wasm".
Closes#13089
There are two places that do it -- commitlog and batchlog replayers. Both can have local system-keyspace reference and use system-keyspace local query-processor for it. The peering save_truncation_record() is not that simple and is not patched by this PR
Closes#13087
* github.com:scylladb/scylladb:
system_keyspace: Unstatic get_truncation_record()
system_keyspace: Unstatic get_truncated_at()
batchlog_manager: Add system_keyspace dependency
main: Swap batchlog manager and system keyspace starts
system_keyspace: Unstatic get_truncated_position()
system_keyspace: Remove unused method
commitlog: Create commitlog_replayer with system keyspace
test: Make cql_test_env::get_system_keyspace() return sharded
commiltlog: Line-up field definitions
Instead of open-coding the same, in an incomplete way.
clear_inactive_reads() does incomplete eviction in severeal ways:
* it doesn't decrement _stats.inactive_reads
* it doesn't set the permit to evicted state
* it doesn't cancel the ttl timer (if any)
* it doesn't call the eviction notifier on the permit (if there is one)
The list goes on. We already have an evict() method that all this
correctly, use that instead of the current badly open-coded alternative.
This patch also enhances the existing test for clear_inactive_reads()
and adds a new one specifically for `stop()` being called while having
inactive reads.
Fixes: #13048Closes#13049
* throw marshal_exception if not the whole string is parsed, we
should error out if the parsed string contains gabage at the end.
before this change, we silent accept uuid like
"ce84997b-6ea2-4468-9f02-8a65abf4wxyz", and parses it as
"ce84997b-6ea2-4468-9f02-8a65abf4". this is not correct.
* throw marshal_exception if stoull() throws,
`stoull()` throws if it fails to parse a string to an unsigned long
long, we should translate the exception to `marshal_exception`, so
we can handle these exception in a consistent manner.
test is updated accordingly.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13069
The manager will need system ks to get truncation record from, so add it
explicitly. Start-stop sequence no allows that
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The replayer code needs system keyspace to fetch truncation records
from, thus it needs this explicit dependency. By the time it runs system
keyspace is fully initialized already
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
- build: cmake: find ANTLR3 before using it
- build: cmake: define FMT_DEPRECATED_OSTREAM
- build: cmake: add include directory for lua
- build: cmake: link redis against db
Closes#13071
* github.com:scylladb/scylladb:
build: cmake: add more tests
build: cmake: find and link against RapidJSON
build: cmake: link couple libraries as whole archive
build: cmake: find ANTLR3 before using it
build: cmake: define FMT_DEPRECATED_OSTREAM
build: cmake: add include directory for lua
build: cmake: link redis against db
the type of return value of `get_table_views()` is a reference, so we
cannot return a reference to a temporary value.
in this change, a member variable is added to hold the _table_schema,
so it can outlive the function call.
this should silence following warning from Clang:
```
test/lib/expr_test_utils.cc:543:16: error: returning reference to local temporary object [-Werror,-Wreturn-stack-address]
return {view_ptr(_table_schema)};
^~~~~~~~~~~~~~~~~~~~~~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Implementation of task_manager's task that covers major keyspace compaction
on one shard.
Closes#12662
* github.com:scylladb/scylladb:
test: extend major keyspace compaction tasks test
compaction: create task manager's task for major keyspace compaction on one shard
- build: cmake: extract more subsystem out into its own CMakeLists.txt
- build: cmake: remove swagger_gen_files
- build: cmake: remove stale TODO comments
- build: cmake: expose scylla_gen_build_dir
- build: cmake: link against cryptopp
- build: cmake: add missing source to utils
- build: cmake: move lib sources into test-lib
- build: cmake: add test/perf
Closes#13059
* github.com:scylladb/scylladb:
build: cmake: add expr_test test
build: cmake: allow test to specify the sources
build: cmake: add test/perf
build: cmake: move lib sources into test-lib
build: cmake: add missing source to utils
build: cmake: link against cryptopp
build: cmake: expose scylla_gen_build_dir
build: cmake: remove stale TODO comments
build: cmake: remove swagger_gen_files
build: cmake: extract more subsystem out into its own CMakeLists.txt
This method requires callers to remember that the sstable is the collection of files on a filesystem and to know what exact directory they are all in. That's not going to work for object storage, instead, sstable should be moved between more abstract states.
This PR replaces move_to_new_dir() call with the change_state() one that accepts target sub-directory string and moves files around. Currently supported state changes:
* staging -> normal
* upload -> normal | staging
* any -> quarantine
All are pretty straightforward and move files between table basedir subdirectories with the exception that upload -> quarantine should move into upload/quarantine subdirectory. Another thing to keep in mind, that normal state doesn't have its subdir but maps directory to table's base directory.
Closes#12648
* github.com:scylladb/scylladb:
sstable: Remove explicit quarantization call
test: Move move_to_new_dir() method from sstable class
sstable, dist.-loader: Introduce and use pick_up_from_upload() method
sstables, code: Introduce and use change_state() call
distributed_loader: Let make_sstables_available choose target directory
some tests are compiled from more source files, so add an extra
parameter, so they can customize the sources.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
due to circular dependency: the .cc files under the root of
project references the symbols defined by the source files under
subdirectories, but the source files under subdirectories also
reference the symbols defined by the .cc files under the root
of project, the targets in test/perf do not compile. but
the general structure is created.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
This small series reorganizes the existing functional tests for aggregation (min, max, count) and adds additional tests for sum reproducing the strange (but Cassandra-compatible) behavior described in issue #13027.
Closes#13038
* github.com:scylladb/scylladb:
cql-pytest: add tests for sum() aggregate
test/cql-pytest: move aggregation tests to one file
The series fixes the `make_nonforwardable` reader, it shouldn't emit `partition_end` for previous partition after `next_partition()` and `fast_forward_to()`
Fixes: #12249Closes#12978
* github.com:scylladb/scylladb:
flat_mutation_reader_test: cleanup, seastar::async -> SEASTAR_THREAD_TEST_CASE
make_nonforwardable: test through run_mutation_source_tests
make_nonforwardable: next_partition and fast_forward_to when single_partition is true
make_forwardable: fix next_partition
flat_mutation_reader_v2: drop forward_buffer_to
nonforwardable reader: fix indentation
nonforwardable reader: refactor, extract reset_partition
nonforwardable reader: add more tests
nonforwardable reader: no partition_end after fast_forward_to()
nonforwardable reader: no partition_end after next_partition()
nonforwardable reader: no partition_end for empty reader
row_cache: pass partition_start though nonforwardable reader
- treewide: do not define/capture unused variables
- sstables/sstables: mark dummy variable for loop [[maybe_unused]]
- util/result_try: reference this explicitly
- raft: reference this explicitly
- idl-compiler: mark captured this used
- build: reenable unused-{variable,lambda-capture} warnings
Closes#12915
* github.com:scylladb/scylladb:
build: reenable unused-{variable,lambda-capture} warnings
test: reader_concurrency_semaphore_test: define target_memory in debug mode
api::failure_detector: mark set_phi_convict_threshold unimplemented
test: memtable_test: mark dummy variable for loop [[maybe_unused]]
idl-compiler: mark captured this used
raft: reference this explicitly
util/result_try: reference this explicitly
sstables/sstables: mark dummy variable for loop [[maybe_unused]]
treewide: do not define/capture unused variables
service: storage_service: clear _node_ops in batch
Since commit 73e258fc34, Scylla has partial
verification for the CLUSTERING ORDER BY clause in CREATE MATERIALIZED
VIEW. Specifically, invalid column names are rejected. But for reasons
explained in issue #12936 and in the test in this patch, Cassandra
demands that if CLUSTERING ORDER BY appears it must list all the
clustering columns, with no duplicates, and do so in the right order.
This patch replaces an existing test which suggested it is fine
(an extention over Cassandra) to accept a partial list of clustering
columns, by a test that verifies that such a partial list, or an
incorrectly-ordered list, or list with duplicates, should be rejected.
The new test fails on Scylla, and passes on Cassandra, so marked as xfail.
Refs #12936.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12938
Before this patch, all scripts which use test/cql-pytest/run.py
looked for the Scylla executable as their first step. This is usually
the right thing to do, except in two cases where Scylla is *not* needed:
1. The script test/cql-pytest/run-cassandra.
2. The script test/alternator/run with the "--aws" option.
So in this patch we change run.py to only look for Scylla when actually
needed (the find_scylla() function is called). In both cases mentioned
above, find_scylla() will never get called and the script can work even
if Scylla was never built.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13010
This flag designates that we should consume only one
partition from the underlying reader. This means that
attempts to move to another partition should cause an EOS.
When next_partition is called, the buffer could
contain partition_start and possibly static_row.
In this case clear_buffer_to_next_partition will
not remove anything from the buffer and the
reader position should not change. Before this patch,
however, we used to set _end_of_stream=false,
which violated the forwardable-reader
contract - the data of the next partition
was emitted after the data of the first partition
without intermediate EOS.
This bug was found when debugging
test_make_nonforwardable_from_mutations_as_mutation_source flakiness.
A corresponding focused test_make_forwardable_next_partition
has been added to exercise this problem.
This patch fixes the problem with method fast_forward_to
which is similar to the one with next_partition, no
partition_end should be injected for the partition if
fast_forward_to was called inside it.
Before the patch, nonforwardable reader injected
partition_end unconditionally. This caused problems
in case next_partition() was called, the downstream
reader might have already injected its own
partition_end marker, and the one from nonforwardable
reader was a duplicate.
Fixes: #12249
The patch introduces the _partition_is_open flag,
inject partition_end only if there was some data
in the input reader.
A simple unit test has been added for
the nonforwardable reader which checks this
new behaviour.
The WASM UDF implementation has changed since the last time the docs
were written. In particular, the Rust helper library has been
released, and using it should be the recommended method.
Some decisions that were only experimental at the start, were also
"set in stone", so we should refer to them as such.
The docs also contain some code examples. This patch adds tests for
these examples to make sure that they are not wrong and misleading.
Closes#12941
small_vector should be feature-wise compatible with std::vector<>,
let's add operator<=> for it.
also, there is not needd to define operator!=() explicitly, C++20
define this for us if operator==() is defined, so let's drop it.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13032
without C++23 `std::ranges::repeat_view`, it'd be cumbersume to
implement a loop without dummy variable. this change helps to
silence following warning:
```
test/boost/memtable_test.cc:1135:26: error: unused variable 'value' [-Werror,-Wunused-variable]
for (int value : boost::irange<int>(0, num_flushes)) {
^
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
these warnings are found by Clang-17 after removing
`-Wno-unused-lambda-capture` and '-Wno-unused-variable' from
the list of disabled warnings in `configure.py`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
This patch adds regression tests for the strange (but Cassandra-compatible)
behavior described in issue #13027 - that sum of no results returns 0
(not null or nothing), and if also asking for p, we get a null there too.
Refs #13027.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
We had separate test files test_minmax.py and test_count.py but the
separate was artificial (and test_count.py even had one test using
min()). Now I that want to add another test for sum(), I don't know
where to put it. So in this patch I combine test_minmax.py and
test_count.py into one test file - test_aggregate.py, and we can
later add sum() tests in the same file.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
test_broadcast_kv_store does not use await or yield at all, so
there is no need to mark it with "asyncio" mark.
tested using
```
SCYLLA_HOME=$HOME/scylla build/cmake/scylla --overprovisioned --developer-mode=yes --consistent-cluster-management=true --experimental-features=broadcast-tables
...
pytest broadcast_tables/test_broadcast_tables.py
```
the test still passes.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13006
