Skip to content

Commit

Permalink
[BACKPORT 2.18][#13956] YCQL: Add error message for GROUP BY clause
Browse files Browse the repository at this point in the history
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
  • Loading branch information
swapshivam3 committed Apr 23, 2024
1 parent a3c9db6 commit 7eb7bf0
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 0 deletions.
6 changes: 6 additions & 0 deletions java/yb-cql/src/test/java/org/yb/cql/TestCreateTable.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import static org.yb.AssertionWrappers.assertTrue;

import java.util.Set;
import java.util.Map;
import java.util.HashMap;

import org.yb.YBTestRunner;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -187,6 +189,10 @@ public void testCreateTableWithColumnNamedOffset() throws Exception {

@Test
public void testCreateTableWithColumnNamedGroup() throws Exception {
// Run test with error suppressed as GROUP BY is not supported.
Map<String, String> flags = new HashMap<>();
flags.put("ycql_suppress_group_by_error", "true");
restartClusterWithTSFlags(flags);
LOG.info("Start test: " + getCurrentTestMethodName());
testCreateTableWithColumnNamedKeyword("group");

Expand Down
16 changes: 16 additions & 0 deletions java/yb-cql/src/test/java/org/yb/cql/TestSelect.java
Original file line number Diff line number Diff line change
Expand Up @@ -2805,4 +2805,20 @@ public void testMultiColumnInSeeks2() throws Exception {
assertEquals(2, metrics.seekCount);
}
}

@Test
public void testGroupBy() throws Exception
{
// Expect error in SELECT with GROUP BY.
session.execute("CREATE TABLE test_tbl (id int primary key, v int);");
runInvalidStmt("SELECT * FROM test_tbl GROUP BY v;");

// Restart with GROUP BY queries with error suppressed.
Map<String, String> flags = new HashMap<>();
flags.put("ycql_suppress_group_by_error", "true");
restartClusterWithTSFlags(flags);

session.execute("CREATE TABLE test_tbl (id int primary key, v int);");
session.execute("SELECT * FROM test_tbl GROUP BY v;");
}
}
7 changes: 7 additions & 0 deletions src/yb/yql/cql/ql/parser/parser_gram.y
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
#include "yb/yql/cql/ql/ptree/pt_transaction.h"

DECLARE_bool(cql_raise_index_where_clause_error);
DECLARE_bool(ycql_suppress_group_by_error);

namespace yb {
namespace ql {
Expand Down Expand Up @@ -203,6 +204,7 @@ using namespace yb::ql;

#define PARSER_INVALID(loc) parser_->Error(loc, ErrorCode::SQL_STATEMENT_INVALID)
#define PARSER_UNSUPPORTED(loc) parser_->Error(loc, ErrorCode::FEATURE_NOT_SUPPORTED)
#define PARSER_UNSUPPORTED_MSG(loc, msg) parser_->Error(loc, msg, ErrorCode::FEATURE_NOT_SUPPORTED)
#define PARSER_NOCODE(loc) parser_->Error(loc, ErrorCode::FEATURE_NOT_YET_IMPLEMENTED)

#define PARSER_CQL_INVALID(loc) parser_->Error(loc, ErrorCode::CQL_STATEMENT_INVALID)
Expand Down Expand Up @@ -2517,6 +2519,11 @@ group_clause:
$$ = nullptr;
}
| GROUP_LA BY group_by_list {
if(!FLAGS_ycql_suppress_group_by_error)
PARSER_UNSUPPORTED_MSG(@1,
"This is not recommended but this error can be suppressed by setting " \
"ycql_suppress_group_by_error flag to false. When set to false, " \
"the error will be suppressed but the GROUP BY clause will be ignored.");
$$ = $3;
}
;
Expand Down
3 changes: 3 additions & 0 deletions src/yb/yql/cql/ql/ptree/pt_select.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ DEFINE_UNKNOWN_bool(enable_uncovered_index_select, true,
"Enable executing select statements using uncovered index");
TAG_FLAG(enable_uncovered_index_select, advanced);

DEFINE_RUNTIME_AUTO_bool(ycql_suppress_group_by_error, kLocalVolatile, true, false,
"Enable to suppress the error raised when using GROUP BY clause");

namespace yb {
namespace ql {

Expand Down

0 comments on commit 7eb7bf0

Please sign in to comment.