Skip to content

Commit

Permalink
fix statements splitting and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@VPolka
VPolka committed Feb 16, 2024
1 parent 0a5fac7 commit d5ddbc2
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 14 deletions.
2 changes: 1 addition & 1 deletion ydb/core/kqp/compile_service/kqp_compile_actor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ class TKqpCompileActor : public TActorBootstrapped<TKqpCompileActor> {
<< ", owner: " << Owner
<< ", statement id: " << statementId);

NYql::TIssue issue(NYql::TPosition(), "Internal error while parsing query.");
NYql::TIssue issue(NYql::TPosition(), "Error while parsing query.");
for (const auto& i : astStatements[statementId].Ast->Issues) {
issue.AddSubIssue(MakeIntrusive<NYql::TIssue>(i));
}
Expand Down
51 changes: 51 additions & 0 deletions ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1918,6 +1918,57 @@ Y_UNIT_TEST_SUITE(KqpQueryService) {
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::UNSUPPORTED, result.GetIssues().ToString());
}

{
// Check parser errors
auto result = db.ExecuteQuery(R"(
UPSERT INTO TestDdl4 (Key, Value) VALUES (2, 2);
SELECT * FROM $a;
UPSERT INTO TestDdl4 (Key, Value) VALUES (3, 3);
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::INTERNAL_ERROR, result.GetIssues().ToString());
UNIT_ASSERT(result.GetIssues().ToOneLineString().Contains("Unknown name: $a"));

result = db.ExecuteQuery(R"(
SELECT * FROM TestDdl4;
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
UNIT_ASSERT_VALUES_EQUAL(result.GetResultSets().size(), 1);
CompareYson(R"([[[1u];[1u]]])", FormatResultSetYson(result.GetResultSet(0)));

result = db.ExecuteQuery(R"(
UPSERT INTO TestDdl4 (Key, Value) VALUES (2, 2);
UPSERT INTO TestDdl4 (Key, Value) VALUES (3, "3");
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString());
UNIT_ASSERT(result.GetIssues().ToOneLineString().Contains("Error: Failed to convert 'Value': String to Optional<Uint64>"));

result = db.ExecuteQuery(R"(
SELECT * FROM TestDdl4;
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
UNIT_ASSERT_VALUES_EQUAL(result.GetResultSets().size(), 1);
CompareYson(R"([[[1u];[1u]];[[2u];[2u]]])", FormatResultSetYson(result.GetResultSet(0)));

result = db.ExecuteQuery(R"(
CREATE TABLE TestDdl5 (
Key Uint64,
Value Uint64,
PRIMARY KEY (Key)
);
UPSERT INTO TestDdl5 (Key, Value) VALUES (1, 1);
UPSERT INTO TestDdl5 (Key, Value) VALUES (3, "3");
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString());
UNIT_ASSERT(result.GetIssues().ToOneLineString().Contains("Error: Failed to convert 'Value': String to Optional<Uint64>"));

result = db.ExecuteQuery(R"(
SELECT * FROM TestDdl5;
)", TTxControl::NoTx()).ExtractValueSync();
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
UNIT_ASSERT_VALUES_EQUAL(result.GetResultSets().size(), 1);
CompareYson(R"([[[1u];[1u]]])", FormatResultSetYson(result.GetResultSet(0)));
}
}
}

Expand Down
1 change: 1 addition & 0 deletions ydb/library/yql/sql/pg/pg_sql.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4483,6 +4483,7 @@ NYql::TAstParseResult PGToYql(const TString& query, const NSQLTranslation::TTran
TVector<NYql::TAstParseResult> results;
TConverter converter(results, settings, query);
NYql::PGParse(query, converter);
Y_ENSURE(!results.empty());
return std::move(results.back());
}

Expand Down
64 changes: 52 additions & 12 deletions ydb/library/yql/sql/v1/sql.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,54 @@ NYql::TAstParseResult SqlToYql(const TString& query, const NSQLTranslation::TTra
}

bool NeedUseForAllStatements(const TRule_sql_stmt_core::AltCase& subquery) {
return subquery == TRule_sql_stmt_core::kAltSqlStmtCore1 || // pragma
subquery == TRule_sql_stmt_core::kAltSqlStmtCore3 || // named nodes
subquery == TRule_sql_stmt_core::kAltSqlStmtCore6 || // use
subquery == TRule_sql_stmt_core::kAltSqlStmtCore12 || // declare
subquery == TRule_sql_stmt_core::kAltSqlStmtCore13 || // import
subquery == TRule_sql_stmt_core::kAltSqlStmtCore14 || // export
subquery == TRule_sql_stmt_core::kAltSqlStmtCore17; // define action or subquery
switch (subquery) {
case TRule_sql_stmt_core::kAltSqlStmtCore1: // pragma
case TRule_sql_stmt_core::kAltSqlStmtCore3: // named nodes
case TRule_sql_stmt_core::kAltSqlStmtCore6: // use
case TRule_sql_stmt_core::kAltSqlStmtCore12: // declare
case TRule_sql_stmt_core::kAltSqlStmtCore13: // import
case TRule_sql_stmt_core::kAltSqlStmtCore14: // export
case TRule_sql_stmt_core::kAltSqlStmtCore18: // define action or subquery
return true;
case TRule_sql_stmt_core::ALT_NOT_SET:
case TRule_sql_stmt_core::kAltSqlStmtCore2: // select
case TRule_sql_stmt_core::kAltSqlStmtCore4: // create table
case TRule_sql_stmt_core::kAltSqlStmtCore5: // drop table
case TRule_sql_stmt_core::kAltSqlStmtCore7: // into table
case TRule_sql_stmt_core::kAltSqlStmtCore8: // commit
case TRule_sql_stmt_core::kAltSqlStmtCore9: // update
case TRule_sql_stmt_core::kAltSqlStmtCore10: // delete
case TRule_sql_stmt_core::kAltSqlStmtCore11: // rollback
case TRule_sql_stmt_core::kAltSqlStmtCore15: // alter table
case TRule_sql_stmt_core::kAltSqlStmtCore16: // alter external table
case TRule_sql_stmt_core::kAltSqlStmtCore17: // do
case TRule_sql_stmt_core::kAltSqlStmtCore19: // if
case TRule_sql_stmt_core::kAltSqlStmtCore20: // for
case TRule_sql_stmt_core::kAltSqlStmtCore21: // values
case TRule_sql_stmt_core::kAltSqlStmtCore22: // create user
case TRule_sql_stmt_core::kAltSqlStmtCore23: // alter user
case TRule_sql_stmt_core::kAltSqlStmtCore24: // create group
case TRule_sql_stmt_core::kAltSqlStmtCore25: // alter group
case TRule_sql_stmt_core::kAltSqlStmtCore26: // drop role
case TRule_sql_stmt_core::kAltSqlStmtCore27: // create object
case TRule_sql_stmt_core::kAltSqlStmtCore28: // alter object
case TRule_sql_stmt_core::kAltSqlStmtCore29: // drop object
case TRule_sql_stmt_core::kAltSqlStmtCore30: // create external data source
case TRule_sql_stmt_core::kAltSqlStmtCore31: // alter external data source
case TRule_sql_stmt_core::kAltSqlStmtCore32: // drop external data source
case TRule_sql_stmt_core::kAltSqlStmtCore33: // create replication
case TRule_sql_stmt_core::kAltSqlStmtCore34: // drop replication
case TRule_sql_stmt_core::kAltSqlStmtCore35: // create topic
case TRule_sql_stmt_core::kAltSqlStmtCore36: // alter topic
case TRule_sql_stmt_core::kAltSqlStmtCore37: // drop topic
case TRule_sql_stmt_core::kAltSqlStmtCore38: // grant permissions
case TRule_sql_stmt_core::kAltSqlStmtCore39: // revoke permissions
case TRule_sql_stmt_core::kAltSqlStmtCore40: // alter table store
case TRule_sql_stmt_core::kAltSqlStmtCore41: // upsert object
case TRule_sql_stmt_core::kAltSqlStmtCore42: // create view
case TRule_sql_stmt_core::kAltSqlStmtCore43: // drop view
return false;
}
}

TVector<NYql::TAstParseResult> SqlToAstStatements(const TString& query, const NSQLTranslation::TTranslationSettings& settings, NYql::TWarningRules* warningRules)
Expand All @@ -144,11 +185,6 @@ TVector<NYql::TAstParseResult> SqlToAstStatements(const TString& query, const NS
if (astProto) {
auto ast = static_cast<const TSQLv1ParserAST&>(*astProto);
const auto& query = ast.GetRule_sql_query();
if (!settings.PerStatementExecution) {
res.emplace_back();
SqlASTToYqlImpl(res.back(), *astProto, ctx);
return res;
}
if (query.Alt_case() == NSQLv1Generated::TRule_sql_query::kAltSqlQuery1) {
std::vector<::NSQLv1Generated::TRule_sql_stmt_core> commonStates;
std::vector<::NSQLv1Generated::TRule_sql_stmt_core> result;
Expand All @@ -159,6 +195,8 @@ TVector<NYql::TAstParseResult> SqlToAstStatements(const TString& query, const NS
TContext ctx(settings, hints, issues);
res.emplace_back();
SqlASTsToYqlsImpl(res.back(), {statements.GetRule_sql_stmt2().GetRule_sql_stmt_core2()}, ctx);
res.back().Issues = std::move(issues);
issues.Clear();
}
for (auto block: statements.GetBlock3()) {
if (NeedUseForAllStatements(block.GetRule_sql_stmt2().GetRule_sql_stmt_core2().Alt_case())) {
Expand All @@ -170,6 +208,8 @@ TVector<NYql::TAstParseResult> SqlToAstStatements(const TString& query, const NS
result = commonStates;
result.push_back(block.GetRule_sql_stmt2().GetRule_sql_stmt_core2());
SqlASTsToYqlsImpl(res.back(), result, ctx);
res.back().Issues = std::move(issues);
issues.Clear();
}
}
} else {
Expand Down
1 change: 0 additions & 1 deletion ydb/library/yql/sql/v1/sql_query.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2727,7 +2727,6 @@ TNodePtr TSqlQuery::Build(const std::vector<::NSQLv1Generated::TRule_sql_stmt_co
}

auto result = BuildQuery(Ctx.Pos(), blocks, true, Ctx.Scoped);
WarnUnusedNodes();
return result;
}
namespace {
Expand Down

0 comments on commit d5ddbc2

Please sign in to comment.