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

SELECT JSON ignores the "AS" specification #8078

Closed
nyh opened this issue Feb 14, 2021 · 4 comments
Closed

SELECT JSON ignores the "AS" specification #8078

nyh opened this issue Feb 14, 2021 · 4 comments
Assignees
@nyh
Labels
area/cassandra 2.2 compatibility area/cql bug user request
Milestone
5.2

Comments

@nyh
Copy link
Contributor

nyh commented Feb 14, 2021 •
edited
Loading

The CQL command SELECT JSON v AS foo, k AS bar FROM table should result in JSON output that looks like {"foo": 0, "bar": 0} - i.e., the names of the columns should be replaced by the names "foo" and "bar" as specified by the "AS" alias clauses in the command. But unfortunately, Scylla ignores these "AS" aliases, and its output is {"v": 0, "k": 0}.

This bug is reproduced by the following tests translated from Cassandra's unit tests:

  • cassandra_tests/validation/entities/json_test.py::testSelectJsonSyntax
  • cassandra_tests/validation/entities/json_test.py::testCaseSensitivity
@slivne slivne added this to the 4.x milestone Feb 15, 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>
@nyh
Copy link
Contributor Author

nyh commented Nov 28, 2022

This issue was just reported by an actual user, who used SELECT JSON with writetime(x) as y, and saw writetime(x) in the output instead of y as expected. So we should probably solve it...

@mykaul mykaul added the high label Nov 28, 2022
@DoronArazii DoronArazii modified the milestones: 5.x, 5.2 Nov 28, 2022
@nyh nyh self-assigned this Nov 28, 2022
@nyh
Copy link
Contributor Author

nyh commented Nov 28, 2022

I have a fix for this already, it's basically two lines of code.

But the two translated Cassandra unit tests that demonstrated this bug unfortunately demonstrate two other bugs as well (#8086 and #8077), so these two tests keep xfailing after this patch as well. I did verify manually that all the assertions related to "AS" which used to fail now pass, but still, these tests don't serve as a good regression test if they continue to xfail - so if they later start to fail even more badly, we won't know.
So I want to add to my patch also a new test that reproduces only this bug and not the two other ones, I'll do this tomorrow.

@nyh nyh added the bug label Nov 28, 2022
nyh added a commit to nyh/scylla that referenced this issue Nov 29, 2022
@nyh
cql: fix column-name aliases in SELECT JSON
316d4f8 
The SELECT JSON statement, just like SELECT, allows the user to rename
selected columns using an "AS" specification. E.g., "SELECT JSON v AS foo".
This specification was not honored: We simply forgot to look at the
alias in SELECT JSON's implementation (we did it correctly in regular
SELECT). So this patch fixes this bug.

We had two tests in cassandra_tests/validation/entities/json_test.py
that reproduced this bug. The checks in those tests now pass, but these
two tests still continue to fail after this patch because of two other
unrelated bugs that were discovered by the same tests. So in this patch
I also add a new test just for this specific issue - to serve as a
regression test.

Fixes scylladb#8078

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
@nyh nyh mentioned this issue Nov 29, 2022
Closed
nyh added a commit to nyh/scylla that referenced this issue Nov 29, 2022
@nyh
cql: fix column-name aliases in SELECT JSON
db59137 
The SELECT JSON statement, just like SELECT, allows the user to rename
selected columns using an "AS" specification. E.g., "SELECT JSON v AS foo".
This specification was not honored: We simply forgot to look at the
alias in SELECT JSON's implementation (we did it correctly in regular
SELECT). So this patch fixes this bug.

We had two tests in cassandra_tests/validation/entities/json_test.py
that reproduced this bug. The checks in those tests now pass, but these
two tests still continue to fail after this patch because of two other
unrelated bugs that were discovered by the same tests. So in this patch
I also add a new test just for this specific issue - to serve as a
regression test.

Fixes scylladb#8078

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

Minor, but can stump newcomers.

avikivity pushed a commit that referenced this issue Dec 5, 2022
@nyh @avikivity
cql: fix column-name aliases in SELECT JSON
0cfb950 
The SELECT JSON statement, just like SELECT, allows the user to rename
selected columns using an "AS" specification. E.g., "SELECT JSON v AS foo".
This specification was not honored: We simply forgot to look at the
alias in SELECT JSON's implementation (we did it correctly in regular
SELECT). So this patch fixes this bug.

We had two tests in cassandra_tests/validation/entities/json_test.py
that reproduced this bug. The checks in those tests now pass, but these
two tests still continue to fail after this patch because of two other
unrelated bugs that were discovered by the same tests. So in this patch
I also add a new test just for this specific issue - to serve as a
regression test.

Fixes #8078

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

Closes #12123

(cherry picked from commit c5121cf)
@avikivity
Copy link
Member

Backported to 5.1 only, which is what newcomers will use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nyh nyh

Labels
area/cassandra 2.2 compatibility area/cql bug user request
Projects
None yet
Milestone
5.2
Development

Successfully merging a pull request may close this issue.

cql: fix column-name aliases in SELECT JSON nyh/scylla
6 participants
@nyh @avikivity @slivne @mykaul @scylladb-promoter @DoronArazii