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

fromJson() should accept "true" and "false" also as strings #7915

Closed
nyh opened this issue Jan 12, 2021 · 2 comments · Fixed by #10134
Closed

fromJson() should accept "true" and "false" also as strings #7915

nyh opened this issue Jan 12, 2021 · 2 comments · Fixed by #10134
Labels
area/cassandra 2.2 compatibility area/cql
Milestone
5.1

Comments

@nyh
Copy link
Contributor

nyh commented Jan 12, 2021 •
edited

When writing to a boolean column with a prepared statement, Cassandra's fromJson() function allows not just JSON's true and false constants, it also allows the strings "true", "false":

stmt = cql.prepare(f"INSERT INTO {table1} (p, b) VALUES (?, fromJson(?))")
cql.execute(stmt, [p, '"true"'])

This capability is not accidental - they even have a special test for it in cassandra_tests/validation/entities/json_test.py::testFromJsonFct.

UPDATE (1 Mar, 2022) @Jadw1 discovered that the following statements about the unprepared case not working in Cassandra were wrong, and I was just misled by a bug in the test. See the next comment for details.

Strangely this test only tests the prepared statement case. Had Cassandra also tested the unprepared statement case, they would have noticed that it doesn't work! While the prepared statement works, the unprepared statement:

cql.execute(f"INSERT INTO {table1} (p, b) VALUES ({p}, '\"true\"')")

Fails with "Invalid STRING constant ("true") for "b" of type boolean". It fails in the same way in Scylla.

But for the prepared statement case, which is allows in Cassandra, Scylla return a a server error, with the strange message "marshaling error: Invalid JSON object "false"". The message is strange, because the JSON string object is not invalid in any way (it is a valid string, and moreover should be convertible into a boolean).

See also test cases in test_json.py: test_fromjson_boolean_string_unprepared - which works on both Scylla and Cassandra, and test_fromjson_boolean_string_prepared which xfails on Scylla.
@slivne slivne added the n00b label Jan 17, 2021
@slivne slivne added this to the 4.5 milestone Jan 17, 2021
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>
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>
@slivne slivne modified the milestones: 4.5, 4.6 Mar 29, 2021
@slivne slivne modified the milestones: 4.6, 4.7 Nov 10, 2021
Jadw1 added a commit to Jadw1/scylla that referenced this issue Feb 24, 2022
@Jadw1
CQL3: fromJson accepts string as bool
6b3dd09 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: scylladb#7915
@Jadw1 Jadw1 mentioned this issue Feb 24, 2022
Merged
Jadw1 added a commit to Jadw1/scylla that referenced this issue Feb 24, 2022
@Jadw1
CQL3/pytest: Updating test_json
921393e 
Referring to issue scylladb#7915, cassandra also works with unprepared
statement. There was missing `fromJson()`, the test was inserting
string into boolean column.
@nyh
Copy link
Contributor Author

nyh commented Mar 1, 2022

@Jadw1 discovered that my "observation" that Cassandra has a bug in the unprepared case, supposedly demonstrated by test_fromjson_boolean_string_unprepared, was completely incorrect, and just caused by a bug in the test - the test was just missing the the fromJson() function call (doh!), so just tried to assign a string to a boolean so unsurprisingly didn't work.

When the test is fixed, Cassandra allows setting a "true" or "false" string JSON to a boolean just fine in either prepared or unprepared statements.

Jadw1 added a commit to Jadw1/scylla that referenced this issue Mar 2, 2022
@Jadw1
CQL3: fromJson accepts string as bool
6d3013d 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: scylladb#7915
Jadw1 added a commit to Jadw1/scylla that referenced this issue Mar 2, 2022
@Jadw1
CQL3/pytest: Updating test_json
78eea75 
Referring to issue scylladb#7915, cassandra also works with unprepared
statement. There was missing `fromJson()`, the test was inserting
string into boolean column.
Jadw1 added a commit to Jadw1/scylla that referenced this issue Mar 4, 2022
@Jadw1
CQL3: fromJson accepts string as bool
5366807 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: scylladb#7915
Jadw1 added a commit to Jadw1/scylla that referenced this issue Mar 4, 2022
@Jadw1
CQL3/pytest: Updating test_json
463b4fb 
Referring to issue scylladb#7915, cassandra also works with unprepared
statement. There was missing `fromJson()`, the test was inserting
string into boolean column.
Jadw1 added a commit to Jadw1/scylla that referenced this issue Mar 4, 2022
@Jadw1
CQL3/pytest: Updating test_json
213dace 
Referring to issue scylladb#7915, cassandra also works with unprepared
statement. There was missing `fromJson()`, the test was inserting
string into boolean column.
nyh added a commit that referenced this issue Mar 8, 2022
@nyh
Merge 'CQL3: fromJson accepts string as bool' from Jadw1
c8152e7 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: #7915

Closes #10134

* github.com:scylladb/scylla:
  CQL3/pytest: Updating test_json
  CQL3: fromJson accepts string as bool
avikivity pushed a commit that referenced this issue Nov 6, 2022
@Jadw1 @avikivity
CQL3: fromJson accepts string as bool
4b66d40 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: #7915
(cherry picked from commit 1902dbc)
avikivity pushed a commit that referenced this issue Nov 6, 2022
@Jadw1 @avikivity
CQL3: fromJson accepts string as bool
4857478 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: #7915
(cherry picked from commit 1902dbc)
@avikivity
Copy link
Member

Backported to 4.6, 5.0. 5.1 has it.

denesb pushed a commit that referenced this issue Nov 7, 2022
@Jadw1 @denesb
CQL3: fromJson accepts string as bool
c0b38c2 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: #7915
(cherry picked from commit 1902dbc)
denesb pushed a commit that referenced this issue Nov 7, 2022
@Jadw1 @denesb
CQL3: fromJson accepts string as bool
4da5fba 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: #7915
(cherry picked from commit 1902dbc)
denesb pushed a commit that referenced this issue Nov 7, 2022
@Jadw1 @denesb
CQL3: fromJson accepts string as bool
e9c7f89 
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: #7915
(cherry picked from commit 1902dbc)
@DoronArazii DoronArazii modified the milestones: 5.0, 5.1 Nov 8, 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/cassandra 2.2 compatibility area/cql
Projects
None yet
Milestone
5.1
Development

Successfully merging a pull request may close this issue.

CQL3: fromJson accepts string as bool Jadw1/scylla
5 participants
@nyh @avikivity @slivne @scylladb-promoter @DoronArazii