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

DELETE/UPDATE print misleading error message suggesting ALLOW FILTERING would work #12474

Closed
nyh opened this issue Jan 8, 2023 · 1 comment
Closed

Comments

@nyh
Copy link
Contributor

nyh commented Jan 8, 2023

If one tries the request

DELETE FROM tbl WHERE partitionKey = ? AND value = ?

Where value is some non-primary-key column, Cassandra refuses to run it saying:

Non PRIMARY KEY columns found in where clause: value

Unfortunately, Scylla gives a very different error message:

Cannot execute this query as it might involve data filtering and thus may have unpredictable performance. If you want to execute this query despite the performance unpredictability, use ALLOW FILTERING

This is error message is unfortunately wrong: It suggests that adding ALLOW FILTERING to this query would make it work. But in fact, it doesn't work - the ALLOW FILTERING isn't even an allowed part of the DELETE syntax. So this error message is wrong and misleading.

By the way, #10593 explains why allowing and supporting ALLOW FILTERING in DELETE might be problematic. If the partition is huge, the DELETE will have to scan it entirely before it can return, because DELETE does not support paging.

I suspect that the "ALLOW FILTERING" message is printed by some common expression-parsing code which assumes it is running for a SELECT. But in this case, it isn't, it is running for a DELETE.

Discovered by the translated Cassandra unit tests:

  • cassandra_tests/validation/operations/compact_storage_test.py::testDeleteWithNoClusteringColumns
  • cassandra_tests/validation/operations/delete_test.py::testDeleteWithNoClusteringColumns
  • cassandra_tests/validation/operations/delete_test.py::testDeleteWithSecondaryIndices

Those tests expect Cassandra's error message. We usually don't insist on having exactly the same error message in Scylla as in Cassandra - but we do insist on the error message not being wrong or misleading, which I think in this case it is.

We have exactly the same bug with other write operations, which also don't support ALLOW FILTERING so shouldn't print errors suggesting that it would help. The following translated Cassandra unit tests reproduce this bug for UPDATE:

  • cassandra_tests/validation/operations/update_test.py::testUpdate
  • cassandra_tests/validation/operations/update_test.py::testUpdateWithTwoClusteringColumns
  • cassandra_tests/validation/operations/update_test.py::testUpdateWithMultiplePartitionKeyComponents
@nyh
Copy link
Contributor Author

nyh commented Jan 8, 2023

Here is an example test showing that the DELETE with a filter on a regular column claims that ALLOW FILTERING is required, but when the user adds this, the statement doesn't work (syntax error):

import pytest
from util import new_test_table, unique_key_int
from cassandra.protocol import InvalidRequest

@pytest.fixture(scope="module")
def table1(cql, test_keyspace):
    with new_test_table(cql, test_keyspace, "p int, c int, v int, PRIMARY KEY (p, c)") as table:
        yield table

def test_delete_partition_with_filter(cql, table1):
    p = unique_key_int()
    cql.execute(f"INSERT INTO {table1} (p, c, v) VALUES ({p}, 1, 10)")
    cql.execute(f"INSERT INTO {table1} (p, c, v) VALUES ({p}, 2, 20)")
    cql.execute(f"INSERT INTO {table1} (p, c, v) VALUES ({p}, 3, 30)")
    with pytest.raises(InvalidRequest, match='ALLOW FILTERING'):
        cql.execute(f"DELETE FROM {table1} WHERE p = {p} AND v > 15")
    # The following request FAILS, with a syntax error :-(
    cql.execute(f"DELETE FROM {table1} WHERE p = {p} AND v > 15 ALLOW FILTERING")

@DoronArazii DoronArazii added this to the 5.x milestone Jan 29, 2023
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 to nyh/scylla that referenced this issue Apr 4, 2023
This is a translation of Cassandra's CQL unit test source file
validation/operations/DeleteText.java into our cql-pytest framework.

There are 51 tests, and they did not reproduce any previously-unknown
bug, but did provide additional reproducers for three known issues:

Refs  scylladb#4244 Add support for mixing token, multi- and single-column
            restrictions

Refs scylladb#12474 DELETE prints misleading error message suggesting ALLOW
            FILTERING would work

Refs scylladb#13250 one-element multi-column restriction should be handled like
            a single-column restriction

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
nyh added a commit to nyh/scylla that referenced this issue Apr 9, 2023
This is a translation of Cassandra's CQL unit test source file
validation/operations/DeleteTest.java into our cql-pytest framework.

There are 51 tests, and they did not reproduce any previously-unknown
bug, but did provide additional reproducers for three known issues:

Refs  scylladb#4244 Add support for mixing token, multi- and single-column
            restrictions

Refs scylladb#12474 DELETE prints misleading error message suggesting ALLOW
            FILTERING would work

Refs scylladb#13250 one-element multi-column restriction should be handled like
            a single-column restriction

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
denesb pushed a commit that referenced this issue Apr 11, 2023
This is a translation of Cassandra's CQL unit test source file
validation/operations/DeleteTest.java into our cql-pytest framework.

There are 51 tests, and they did not reproduce any previously-unknown
bug, but did provide additional reproducers for three known issues:

Refs  #4244 Add support for mixing token, multi- and single-column
            restrictions

Refs #12474 DELETE prints misleading error message suggesting ALLOW
            FILTERING would work

Refs #13250 one-element multi-column restriction should be handled like
            a single-column restriction

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

Closes #13436
@nyh nyh changed the title DELETE prints misleading error message suggesting ALLOW FILTERING would work DELETE/UPDATE print misleading error message suggesting ALLOW FILTERING would work Jun 12, 2023
nyh added a commit to nyh/scylla that referenced this issue Jun 13, 2023
This is a translation of Cassandra's CQL unit test source file
validation/operations/UpdateTest.java into our cql-pytest framework.

There are 18 tests, and they did not reproduce any previously-unknown
bug, but did provide additional reproducers for two known issues:

Refs scylladb#12243: Setting USING TTL of "null" should be allowed

Refs scylladb#12474: DELETE/UPDATE print misleading error message suggesting
             ALLOW FILTERING would work
     Note that we knew about this issue for the DELETE operation, and
     the new test shows the same issue exists for UPDATE.

I had to modify some of the tests to allow for different error messages
in ScyllaDB (in cases where the different message makes sense), as well
as cases where we decided to allow in Scylla some behaviors that are
forbidden in Cassandra - namely Refs scylladb#12472.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
denesb pushed a commit that referenced this issue Jun 14, 2023
This is a translation of Cassandra's CQL unit test source file
validation/operations/UpdateTest.java into our cql-pytest framework.

There are 18 tests, and they did not reproduce any previously-unknown
bug, but did provide additional reproducers for two known issues:

Refs #12243: Setting USING TTL of "null" should be allowed

Refs #12474: DELETE/UPDATE print misleading error message suggesting
             ALLOW FILTERING would work
     Note that we knew about this issue for the DELETE operation, and
     the new test shows the same issue exists for UPDATE.

I had to modify some of the tests to allow for different error messages
in ScyllaDB (in cases where the different message makes sense), as well
as cases where we decided to allow in Scylla some behaviors that are
forbidden in Cassandra - namely Refs #12472.

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

Closes #14222
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.
Jager227 added a commit to soft-stech/scylladb that referenced this issue Oct 23, 2023
…filtering" error to pop up in delete/update statements

Fixes scylladb#12474
Jager227 added a commit to soft-stech/scylladb that referenced this issue Oct 26, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
@mykaul mykaul modified the milestones: Backlog, 6.0 Oct 26, 2023
Jager227 added a commit to soft-stech/scylladb that referenced this issue Nov 8, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
Jager227 added a commit to soft-stech/scylladb that referenced this issue Nov 15, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
Jager227 added a commit to soft-stech/scylladb that referenced this issue Nov 22, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
Jager227 added a commit to soft-stech/scylladb that referenced this issue Dec 6, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
Jager227 added a commit to soft-stech/scylladb that referenced this issue Dec 6, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
Jager227 added a commit to soft-stech/scylladb that referenced this issue Dec 7, 2023
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474
dgarcia360 pushed a commit to dgarcia360/scylla that referenced this issue Apr 30, 2024
…filtering" error to pop up in delete/update statements

Modified Cassandra tests to check for Scylla's error messages
Fixes scylladb#12474

Closes scylladb#15811
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants