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.

Sign up for GitHub

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

Jump to bottom

INSERT JSON with bad arguments should yield InvalidRequest error, not internal error #8085

Open
nyh opened this issue Feb 15, 2021 · 0 comments
Open

INSERT JSON with bad arguments should yield InvalidRequest error, not internal error #8085

nyh opened this issue Feb 15, 2021 · 0 comments
Labels
area/cql P4 Low Priority
Milestone
Backlog

Comments

@nyh
Copy link
Contributor

nyh commented Feb 15, 2021

Right now, attempting an INSERT JSON statement with a bad argument - something which is not a string containing a JSON map - often yields an internal server error instead of the expected invalid request. For example,

execute(cql, table, "INSERT INTO %s JSON ?", None)

Yields on Scylla:

E   cassandra.cluster.NoHostAvailable: ('Unable to complete the operation against any hosts', {<Host: 127.18.80.248:9042 datacenter1>: <Error from server: code=0000 [Server error] message="bad optional access">})

I.e., an internal error which made the driver think a different node should be tried, while in Cassandra the driver was told - as expected - that the query was invalid:

E   cassandra.InvalidRequest: Error from server: code=2200 [Invalid query] message="Got null for INSERT JSON values"

Scylla should behave like Cassandra in this.

The translated Cassandra test cassandra_tests/validation/entities/json_test.py::testInsertJsonSyntax reproduces several variants of this problem, with different bad argument types.
avikivity pushed a commit that referenced this issue Feb 16, 2021
@nyh @avikivity
cql-pytest: port Cassandra's unit test validation/entities/json_test
655fdea 
In this patch, we port validation/entities/json_test.java, containing
21 tests for various JSON-related operations - SELECT JSON, INSERT JSON,
and the fromJson() and toJson() functions.

In porting these tests, I uncovered 19 (!!) previously unknown bugs in
Scylla:

Refs #7911: Failed fromJson() should result in FunctionFailure error, not
            an internal error.
Refs #7912: fromJson() should allow null parameter.
Refs #7914: fromJson() integer overflow should cause an error, not silent
            wrap-around.
Refs #7915: fromJson() should accept "true" and "false" also as strings.
Refs #7944: fromJson() should not accept the empty string "" as a number.
Refs #7949: fromJson() fails to set a map<ascii, int>.
Refs #7954: fromJson() fails to set null tuple elements.
Refs #7972: toJson() truncates some doubles to integers.
Refs #7988: toJson() produces invalid JSON for columns with "time" type.
Refs #7997: toJson() is missing a timezone on timestamp.
Refs #8001: Documented unit "µs" not supported for assigning a "duration"
            type.
Refs #8002: toJson() of decimal type doesn't use exponents so can produce
            huge output.
Refs #8077: SELECT JSON output for function invocations should be
            compatible with Cassandra.
Refs #8078: SELECT JSON ignores the "AS" specification.
Refs #8085: INSERT JSON with bad arguments should yield InvalidRequest
            error, not internal error.
Refs #8086: INSERT JSON cannot handle user-defined types with case-
            sensitive component names.
Refs #8087: SELECT JSON incorrectly quotes strings inside map keys.
Refs #8092: SELECT JSON missing null component after adding field to
            UDT definition.
Refs #8100: SELECT JSON with IN and ORDER BY does not obey the ORDER BY.

Due to these bugs, 9 out of the 21 tests here currently xfail.

As usual in these sort of tests, all 21 tests pass when running against
Cassandra.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210216154858.1172313-1-nyh@scylladb.com>
@slivne slivne added this to the 4.x milestone Feb 18, 2021
avikivity pushed a commit that referenced this issue Feb 18, 2021
@nyh @avikivity
cql-pytest: port Cassandra's unit test validation/entities/json_test
02dde2a 
In this patch, we port validation/entities/json_test.java, containing
21 tests for various JSON-related operations - SELECT JSON, INSERT JSON,
and the fromJson() and toJson() functions.

In porting these tests, I uncovered 19 (!!) previously unknown bugs in
Scylla:

Refs #7911: Failed fromJson() should result in FunctionFailure error, not
            an internal error.
Refs #7912: fromJson() should allow null parameter.
Refs #7914: fromJson() integer overflow should cause an error, not silent
            wrap-around.
Refs #7915: fromJson() should accept "true" and "false" also as strings.
Refs #7944: fromJson() should not accept the empty string "" as a number.
Refs #7949: fromJson() fails to set a map<ascii, int>.
Refs #7954: fromJson() fails to set null tuple elements.
Refs #7972: toJson() truncates some doubles to integers.
Refs #7988: toJson() produces invalid JSON for columns with "time" type.
Refs #7997: toJson() is missing a timezone on timestamp.
Refs #8001: Documented unit "µs" not supported for assigning a "duration"
            type.
Refs #8002: toJson() of decimal type doesn't use exponents so can produce
            huge output.
Refs #8077: SELECT JSON output for function invocations should be
            compatible with Cassandra.
Refs #8078: SELECT JSON ignores the "AS" specification.
Refs #8085: INSERT JSON with bad arguments should yield InvalidRequest
            error, not internal error.
Refs #8086: INSERT JSON cannot handle user-defined types with case-
            sensitive component names.
Refs #8087: SELECT JSON incorrectly quotes strings inside map keys.
Refs #8092: SELECT JSON missing null component after adding field to
            UDT definition.
Refs #8100: SELECT JSON with IN and ORDER BY does not obey the ORDER BY.

Due to these bugs, 8 out of the 21 tests here currently xfail and one
has to be skipped (issue #8100 causes the sanitizer to detect a use
after free, and crash Scylla).

As usual in these sort of tests, all 21 tests pass when running against
Cassandra.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210217130732.1202811-1-nyh@scylladb.com>
@DoronArazii DoronArazii added P4 Low Priority and removed low labels Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/cql P4 Low Priority
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@nyh @slivne @DoronArazii