-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Unsupported empty clustering key in COMPACT table #12749
Milestone
Comments
nyh
added
symptom/ux
Concerns regarding the user experience in working with Scylla.
area/cql
labels
Feb 6, 2023
nyh
added a commit
to nyh/scylla
that referenced
this issue
Feb 6, 2023
This patch adds two additional tests for empty (e.g., empty string) clustering keys. The first test disproves a worry that was raised in scylladb#12561 that perhaps empty clustering keys only seem work, but they don't get written to sstables. The new test verifies that there is no bug - they are written and can be read correctly. The second test reproduces issue scylladb#12749 - an empty clustering key is not allowed in a COMPACT STORAGE table, as expected (it is also not allowed in Cassandra) - but the error message in that case is wrong. Refs scylladb#12561 Refs scylladb#12749 Signed-off-by: Nadav Har'El <nyh@scylladb.com>
nyh
added
area/cassandra 3.x compatibility
and removed
symptom/ux
Concerns regarding the user experience in working with Scylla.
labels
Feb 6, 2023
nyh
changed the title
Writing empty clustering key to COMPACT table results in a misleading error message
Unsupported empty clustering key in COMPACT table
Feb 6, 2023
nyh
added a commit
to nyh/scylla
that referenced
this issue
Feb 6, 2023
This patch adds three additional tests for empty (e.g., empty string) clustering keys. The first test disproves a worry that was raised in scylladb#12561 that perhaps empty clustering keys only seem work, but they don't get written to sstables. The new test verifies that there is no bug - they are written and can be read correctly. The second and third test reproduce issue scylladb#12749 - an empty clustering should be allowed in a COMPACT STORAGE table only if there is a compound (multi-column) clustering key. But as the tests demonstrate, 1. if there is just one clustering column, Scylla gives the wrong error message, and 2. if there is a compound clustering key, Scylla doesn't allow an empty key as it should. As usual, all tests pass on Cassandra. The last two tests fail on Scylla, so are marked xfail. Refs scylladb#12561 Refs scylladb#12749 Signed-off-by: Nadav Har'El <nyh@scylladb.com>
denesb
pushed a commit
that referenced
this issue
Feb 7, 2023
This patch adds three additional tests for empty (e.g., empty string) clustering keys. The first test disproves a worry that was raised in #12561 that perhaps empty clustering keys only seem work, but they don't get written to sstables. The new test verifies that there is no bug - they are written and can be read correctly. The second and third test reproduce issue #12749 - an empty clustering should be allowed in a COMPACT STORAGE table only if there is a compound (multi-column) clustering key. But as the tests demonstrate, 1. if there is just one clustering column, Scylla gives the wrong error message, and 2. if there is a compound clustering key, Scylla doesn't allow an empty key as it should. As usual, all tests pass on Cassandra. The last two tests fail on Scylla, so are marked xfail. Refs #12561 Refs #12749 Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #12750
denesb
pushed a commit
that referenced
this issue
Feb 7, 2023
This patch adds three additional tests for empty (e.g., empty string) clustering keys. The first test disproves a worry that was raised in #12561 that perhaps empty clustering keys only seem work, but they don't get written to sstables. The new test verifies that there is no bug - they are written and can be read correctly. The second and third test reproduce issue #12749 - an empty clustering should be allowed in a COMPACT STORAGE table only if there is a compound (multi-column) clustering key. But as the tests demonstrate, 1. if there is just one clustering column, Scylla gives the wrong error message, and 2. if there is a compound clustering key, Scylla doesn't allow an empty key as it should. As usual, all tests pass on Cassandra. The last two tests fail on Scylla, so are marked xfail. Refs #12561 Refs #12749 Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #12750
nyh
added a commit
to nyh/scylla
that referenced
this issue
Feb 12, 2023
This is a translation of Cassandra's CQL unit test source file validation/operations/CompactStorageTest.java into our cql-pytest framework. This very large test file includes 86 tests for various types of operations and corner cases of WITH COMPACT STORAGE tables. All 86 tests pass on Cassandra (except one using a deprecated feature that needs to be specially enabled). 30 of the tests fail on Scylla reproducing 7 already-known Scylla issues and 7 previously-unknown issues: Already known issues: Refs scylladb#3882: Support "ALTER TABLE DROP COMPACT STORAGE" Refs scylladb#4244: Add support for mixing token, multi- and single-column restrictions Refs scylladb#5361: LIMIT doesn't work when using GROUP BY Refs scylladb#5362: LIMIT is not doing it right when using GROUP BY Refs scylladb#5363: PER PARTITION LIMIT doesn't work right when using GROUP BY Refs scylladb#7735: CQL parser missing support for Cassandra 3.10's new "+=" syntax Refs scylladb#8627: Cleanly reject updates with indexed values where value > 64k New issues: Refs scylladb#12471: Range deletions on COMPACT STORAGE is not supported Refs scylladb#12474: DELETE prints misleading error message suggesting ALLOW FILTERING would work Refs scylladb#12477: Combination of COUNT with GROUP BY is different from Cassandra in case of no matches Refs scylladb#12479: SELECT DISTINCT should refuse GROUP BY with clustering column Refs scylladb#12526: Support filtering on COMPACT tables Refs scylladb#12749: Unsupported empty clustering key in COMPACT table Refs scylladb#12815: Hidden column "value" in compact table isn't completely hidden Signed-off-by: Nadav Har'El <nyh@scylladb.com>
denesb
pushed a commit
that referenced
this issue
Feb 20, 2023
This is a translation of Cassandra's CQL unit test source file validation/operations/CompactStorageTest.java into our cql-pytest framework. This very large test file includes 86 tests for various types of operations and corner cases of WITH COMPACT STORAGE tables. All 86 tests pass on Cassandra (except one using a deprecated feature that needs to be specially enabled). 30 of the tests fail on Scylla reproducing 7 already-known Scylla issues and 7 previously-unknown issues: Already known issues: Refs #3882: Support "ALTER TABLE DROP COMPACT STORAGE" Refs #4244: Add support for mixing token, multi- and single-column restrictions Refs #5361: LIMIT doesn't work when using GROUP BY Refs #5362: LIMIT is not doing it right when using GROUP BY Refs #5363: PER PARTITION LIMIT doesn't work right when using GROUP BY Refs #7735: CQL parser missing support for Cassandra 3.10's new "+=" syntax Refs #8627: Cleanly reject updates with indexed values where value > 64k New issues: Refs #12471: Range deletions on COMPACT STORAGE is not supported Refs #12474: DELETE prints misleading error message suggesting ALLOW FILTERING would work Refs #12477: Combination of COUNT with GROUP BY is different from Cassandra in case of no matches Refs #12479: SELECT DISTINCT should refuse GROUP BY with clustering column Refs #12526: Support filtering on COMPACT tables Refs #12749: Unsupported empty clustering key in COMPACT table Refs #12815: Hidden column "value" in compact table isn't completely hidden Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #12816
tgrabiec
pushed a commit
that referenced
this issue
Feb 20, 2023
This is a translation of Cassandra's CQL unit test source file validation/operations/CompactStorageTest.java into our cql-pytest framework. This very large test file includes 86 tests for various types of operations and corner cases of WITH COMPACT STORAGE tables. All 86 tests pass on Cassandra (except one using a deprecated feature that needs to be specially enabled). 30 of the tests fail on Scylla reproducing 7 already-known Scylla issues and 7 previously-unknown issues: Already known issues: Refs #3882: Support "ALTER TABLE DROP COMPACT STORAGE" Refs #4244: Add support for mixing token, multi- and single-column restrictions Refs #5361: LIMIT doesn't work when using GROUP BY Refs #5362: LIMIT is not doing it right when using GROUP BY Refs #5363: PER PARTITION LIMIT doesn't work right when using GROUP BY Refs #7735: CQL parser missing support for Cassandra 3.10's new "+=" syntax Refs #8627: Cleanly reject updates with indexed values where value > 64k New issues: Refs #12471: Range deletions on COMPACT STORAGE is not supported Refs #12474: DELETE prints misleading error message suggesting ALLOW FILTERING would work Refs #12477: Combination of COUNT with GROUP BY is different from Cassandra in case of no matches Refs #12479: SELECT DISTINCT should refuse GROUP BY with clustering column Refs #12526: Support filtering on COMPACT tables Refs #12749: Unsupported empty clustering key in COMPACT table Refs #12815: Hidden column "value" in compact table isn't completely hidden Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #12816
nyh
added a commit
that referenced
this issue
Aug 13, 2023
This is a translation of Cassandra's CQL unit test source file validation/operations/CompactStorageTest.java into our cql-pytest framework. This very large test file includes 86 tests for various types of operations and corner cases of WITH COMPACT STORAGE tables. All 86 tests pass on Cassandra (except one using a deprecated feature that needs to be specially enabled). 30 of the tests fail on Scylla reproducing 7 already-known Scylla issues and 7 previously-unknown issues: Already known issues: Refs #3882: Support "ALTER TABLE DROP COMPACT STORAGE" Refs #4244: Add support for mixing token, multi- and single-column restrictions Refs #5361: LIMIT doesn't work when using GROUP BY Refs #5362: LIMIT is not doing it right when using GROUP BY Refs #5363: PER PARTITION LIMIT doesn't work right when using GROUP BY Refs #7735: CQL parser missing support for Cassandra 3.10's new "+=" syntax Refs #8627: Cleanly reject updates with indexed values where value > 64k New issues: Refs #12471: Range deletions on COMPACT STORAGE is not supported Refs #12474: DELETE prints misleading error message suggesting ALLOW FILTERING would work Refs #12477: Combination of COUNT with GROUP BY is different from Cassandra in case of no matches Refs #12479: SELECT DISTINCT should refuse GROUP BY with clustering column Refs #12526: Support filtering on COMPACT tables Refs #12749: Unsupported empty clustering key in COMPACT table Refs #12815: Hidden column "value" in compact table isn't completely hidden Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #12816 (cherry picked from commit 328cdb2)
nyh
added a commit
that referenced
this issue
Aug 13, 2023
This is a translation of Cassandra's CQL unit test source file validation/operations/CompactStorageTest.java into our cql-pytest framework. This very large test file includes 86 tests for various types of operations and corner cases of WITH COMPACT STORAGE tables. All 86 tests pass on Cassandra (except one using a deprecated feature that needs to be specially enabled). 30 of the tests fail on Scylla reproducing 7 already-known Scylla issues and 7 previously-unknown issues: Already known issues: Refs #3882: Support "ALTER TABLE DROP COMPACT STORAGE" Refs #4244: Add support for mixing token, multi- and single-column restrictions Refs #5361: LIMIT doesn't work when using GROUP BY Refs #5362: LIMIT is not doing it right when using GROUP BY Refs #5363: PER PARTITION LIMIT doesn't work right when using GROUP BY Refs #7735: CQL parser missing support for Cassandra 3.10's new "+=" syntax Refs #8627: Cleanly reject updates with indexed values where value > 64k New issues: Refs #12471: Range deletions on COMPACT STORAGE is not supported Refs #12474: DELETE prints misleading error message suggesting ALLOW FILTERING would work Refs #12477: Combination of COUNT with GROUP BY is different from Cassandra in case of no matches Refs #12479: SELECT DISTINCT should refuse GROUP BY with clustering column Refs #12526: Support filtering on COMPACT tables Refs #12749: Unsupported empty clustering key in COMPACT table Refs #12815: Hidden column "value" in compact table isn't completely hidden Signed-off-by: Nadav Har'El <nyh@scylladb.com> Closes #12816 (cherry picked from commit 328cdb2) (cherry picked from commit e11561e) Modified for 5.1 to comment out error-path tests for "unset" values what are silently ignored (instead of being detected) in this version.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
An empty clustering key (e.g., an empty string) is allowed in normal tables, and we have several tests for that in test/cql-pytest/test_empty.py. However, it turns out that in Cassandra the support for an empty clustering key in a table WITH COMPACT STORAGE is more subtle:
Invalid empty or null value for column c
Scylla gets both cases wrong:
In the first case (single-column clustering key c), Scylla gives a misleading error message:
Missing PRIMARY KEY part c
, which doesn't say what went wrong (since "c" was not missing in the write, it was just an empty string).In the second case (compound clustering key c1,c2), Scylla gives this same error message
Missing PRIMARY KEY part c1
when c1 is set to an empty string - instead of supporting this case as Cassandra does.By the way, as far as I can tell, this bizarre situation isn't documented anywhere as far as I can tell, and I don't know its justification (maybe there is no real justification - we have exactly the same situation in normal tables for empty single-column partition keys which CQL forbids but allows empty partition-key components).
This issue is reproduced by our tests:
test_empty.py
::test_insert_empty_string_key_compact
(the first case, just a wrong error message)test_empty.py
::test_insert_empty_string_compound_clustering_key_compact
(the second case, more serious)And also the following tests translated from the Cassandra test suite, in
cassandra_tests/validation/operations/compact_storage_test.py
::testEmptyRestrictionValue
testEmptyRestrictionValueWithMultipleClusteringColumns
testEmptyRestrictionValueWithOrderBy
testEmptyRestrictionValueWithMultipleClusteringColumnsAndOrderBy
The text was updated successfully, but these errors were encountered: