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

YCQL -- Disallow GROUP BY syntax #13956

Open
yugabyte-ci opened this issue Sep 9, 2022 · 2 comments
Open

YCQL -- Disallow GROUP BY syntax #13956

yugabyte-ci opened this issue Sep 9, 2022 · 2 comments
Assignees
Labels
area/ycql Yugabyte CQL (YCQL) good first issue This is a good issue to start contributing! jira-originated kind/enhancement This is an enhancement of an existing feature priority/high High Priority

Comments

@yugabyte-ci
Copy link
Contributor

yugabyte-ci commented Sep 9, 2022

Jira Link: DB-3446

We don't support the GROUP BY clause in YCQL (only in YSQL) but it is allowed in the syntax (presumably by mistake).

I think the issue is that the group_clause was not disallowed in the parser:
https://github.com/yugabyte/yugabyte-db/blob/master/src/yb/yql/cql/ql/parser/parser_gram.y#L2519 (see for instance the PARSER_UNSUPPORTED calls for some of the other sub-cases below -- the should be there for the top-level non-empty case).

In terms of functionality, from this code: https://github.com/yugabyte/yugabyte-db/blob/master/src/yb/yql/cql/ql/parser/parser_gram.y#L2178 it looks like we are just ignoring the GROUP BY clause if set.

Finally, if backwards compatibility is an issue we can make this a warning rather than an error by default (for GROUP BY specifically.

@yugabyte-ci yugabyte-ci added jira-originated kind/new-feature This is a request for a completely new feature priority/low Low priority status/awaiting-triage Issue awaiting triage labels Sep 9, 2022
@skahler-yuga
Copy link

skahler-yuga commented Sep 9, 2022

A simple test case this would be highlighted here

https://howtoprogram.xyz/2017/02/18/using-group-apache-cassandara/

CREATE TABLE temperature_by_day (
            weatherstation_id text,
            date text,
            event_time timestamp,
            temperature float,
            PRIMARY KEY ((weatherstation_id,date),event_time)
         );
INSERT INTO temperature_by_day(weatherstation_id,date,event_time,temperature)
         VALUES ('1234WXYZ','2016-04-03','2016-04-03 07:01:00',73);

INSERT INTO temperature_by_day(weatherstation_id,date,event_time,temperature)
         VALUES ('1234WXYZ','2016-04-03','2016-04-03 07:02:00',70);

INSERT INTO temperature_by_day(weatherstation_id,date,event_time,temperature)
         VALUES ('1234WXYZ','2016-04-04','2016-04-04 07:01:00',73);
INSERT INTO temperature_by_day(weatherstation_id,date,event_time,temperature)
         VALUES ('1234WXYZ','2016-04-04','2016-04-04 07:02:00',74);

SELECT weatherstation_id, date, MAX(temperature) FROM temperature_by_day GROUP BY weatherstation_id, date;
SyntaxException: Invalid CQL Statement. Selecting aggregate together with rows of non-aggregate values is not allowed
SELECT weatherstation_id, date, MAX(temperature) FROM temperature_by_day
       ^^^^^^^^^^^^^^^^^
                                 GROUP BY weatherstation_id, date;
 (ql error -12)


SELECT MAX(temperature) FROM temperature_by_day GROUP BY weatherstation_id, date;

system.max(temperature)
                      74

(1 rows)

@kmuthukk kmuthukk added the good first issue This is a good issue to start contributing! label Sep 9, 2022
@yugabyte-ci yugabyte-ci added area/ycql Yugabyte CQL (YCQL) kind/bug This issue is a bug and removed kind/new-feature This is a request for a completely new feature labels Sep 13, 2022
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature and removed kind/bug This issue is a bug status/awaiting-triage Issue awaiting triage labels Oct 12, 2022
@SiyaoIsHiding
Copy link

I think I can fix this. @kmuthukk May you please give any hints, e.g. file name, class name, function name, that will help me navigate to the cause of the issue?

@yugabyte-ci yugabyte-ci added priority/high High Priority and removed priority/low Low priority labels Dec 14, 2023
swapshivam3 added a commit that referenced this issue Apr 16, 2024
Summary:
GROUP BY clause is not supported by YCQL, but is allowed in the parser currently, and does not have any effect on the SELECT query. This revision adds an
error message when using GROUP BY clause and also an autoflag ycql_suppress_group_by_error to suppress the error in case of any compatibility issues. Promoting the autoflag makes YCQLsh start throwing the error.
Jira: DB-3446

Test Plan:
```
./yb_build.sh --java-test org.yb.cql.TestCreateTable#testCreateTableWithColumnNamedGroup
./yb_build.sh --java-test org.yb.cql.TestSelect#testGroupBy
```

Reviewers: skumar, stiwary, loginov

Reviewed By: skumar, stiwary

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D33977
swapshivam3 added a commit that referenced this issue Apr 23, 2024
Summary:
Original commit: 80def06 / D33977
GROUP BY clause is not supported by YCQL, but is allowed in the parser currently, and does not have any effect on the SELECT query. This revision adds an
error message when using GROUP BY clause and also an autoflag ycql_suppress_group_by_error to suppress the error in case of any compatibility issues. Promoting the autoflag makes YCQLsh start throwing the error.
Jira: DB-3446

Test Plan:
```
./yb_build.sh --java-test org.yb.cql.TestCreateTable#testCreateTableWithColumnNamedGroup
./yb_build.sh --java-test org.yb.cql.TestSelect#testGroupBy
```

Reviewers: skumar, stiwary, loginov

Reviewed By: skumar

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34159
swapshivam3 added a commit that referenced this issue Apr 23, 2024
Summary:
Original commit: 80def06 / D33977
GROUP BY clause is not supported by YCQL, but is allowed in the parser currently, and does not have any effect on the SELECT query. This revision adds an
error message when using GROUP BY clause and also an autoflag ycql_suppress_group_by_error to suppress the error in case of any compatibility issues. Promoting the autoflag makes YCQLsh start throwing the error.
Jira: DB-3446

Test Plan:
```
./yb_build.sh --java-test org.yb.cql.TestCreateTable#testCreateTableWithColumnNamedGroup
./yb_build.sh --java-test org.yb.cql.TestSelect#testGroupBy
```

Reviewers: skumar, stiwary, loginov

Reviewed By: skumar

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34158
swapshivam3 added a commit that referenced this issue Apr 23, 2024
Summary:
Original commit: 80def06 / D33977
GROUP BY clause is not supported by YCQL, but is allowed in the parser currently, and does not have any effect on the SELECT query. This revision adds an
error message when using GROUP BY clause and also an autoflag ycql_suppress_group_by_error to suppress the error in case of any compatibility issues. Promoting the autoflag makes YCQLsh start throwing the error.
Jira: DB-3446

Test Plan:
```
./yb_build.sh --java-test org.yb.cql.TestCreateTable#testCreateTableWithColumnNamedGroup
./yb_build.sh --java-test org.yb.cql.TestSelect#testGroupBy
```

Reviewers: skumar, stiwary, loginov

Reviewed By: skumar

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ycql Yugabyte CQL (YCQL) good first issue This is a good issue to start contributing! jira-originated kind/enhancement This is an enhancement of an existing feature priority/high High Priority
Projects
None yet
Development

No branches or pull requests

5 participants