Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/cql-pytest: add simple tests for COUNT aggregation #12478

Closed
wants to merge 1 commit into from

Conversation

nyh
Copy link
Contributor

@nyh nyh commented Jan 9, 2023

This patch adds a few simple functional test for the COUNT aggregation feature, and in particular how count(*) differs from count(v), and how COUNT interacts with the GROUP BY operation.

3 of the 8 new tests are marked xfail, and reproduce two newly discovered issues:

Refs #12475: Global COUNT with empty IN results in an internal error
instead of the expected empty count.

Refs #12477: Combining COUNT with GROUP by results with empty results
in Cassandra, and one result with empty count in Scylla.

Signed-off-by: Nadav Har'El nyh@scylladb.com

@nyh nyh requested review from Jadw1, cvybhu and havaker January 9, 2023 14:06
@scylladb-promoter
Copy link
Contributor

@nyh
Copy link
Contributor Author

nyh commented Jan 16, 2023

The new test caused Scylla to crash in a weird way:

ERROR 2023-01-09 18:22:56,247 [shard 0] forward_service - forward_aggregates::finalize(): operation cannot be completed due to invalid argument sizes. 

I rebased on new master hoping maybe the crash was something known and already fixed, but if not, I'll need to debug it. Maybe the "internal error" I saw (#12475) can in some circumstances cause a crash?

This patch adds a few simple functional test for the COUNT aggregation
feature, and in particular how count(*) differs from count(v), and how
COUNT interacts with the GROUP BY operation.

3 of the 8 new tests are marked xfail, and reproduce two newly
discovered issues:

Refs scylladb#12475: Global COUNT with empty IN results in an internal error
             instead of the expected empty count.

Refs scylladb#12477: Combining COUNT with GROUP by results with empty results
             in Cassandra, and one result with empty count in Scylla.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
@scylladb-promoter
Copy link
Contributor

@avikivity
Copy link
Member

/cc @havaker @wmitros

@nyh
Copy link
Contributor Author

nyh commented Jan 16, 2023

Hmm, yes :-( It turns out that the test for #12475 (test_count_empty_in), which in dev mode causes "just" an internal error, in debug mode causes Scylla to crash. If I want to add these tests now I'll have to "skip" this test, but maybe it's better to just fix 12475 first, as it just got a little bit worse (it can be used to crash Scylla).

The failing test is simply running:

select count(*) from tbl where p in ()

This silly query, with an empty "in", causes Scylla to crash in debug mode. The error is
ERROR 2023-01-16 18:40:21,775 [shard 0] forward_service - forward_aggregates::finalize(): operation cannot be completed due to invalid argument sizes. this.aggrs.size(): 1 result.query_result.size(): 0.
You can see the full stack trace in https://jenkins.scylladb.com/job/releng/job/Scylla-CI/3857/testReport/junit/(root)/non-boost%20tests/cql_pytest_test_count_debug_1/

@nyh
Copy link
Contributor Author

nyh commented Feb 2, 2023

I'm closing PR and will open a new one which will fix one of the bugs that the test found - that cause Scylla to crash in debug mode (because of an "on_internal_error") and the CI to fail.

@nyh nyh closed this Feb 2, 2023
@nyh
Copy link
Contributor Author

nyh commented Feb 2, 2023

Opened #12715 to replace this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants