Skip to content

Commit

Permalink
Revert file cycles detection (ydb-platform#4802)
Browse files Browse the repository at this point in the history
  • Loading branch information
@vitstn
vitstn authored May 24, 2024
1 parent 13b74c9 commit 9529d2c
Show file tree
Hide file tree
Showing 8 changed files with 8 additions and 83 deletions.
50 changes: 8 additions & 42 deletions ydb/library/yql/providers/config/yql_config_provider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ namespace {
for (auto& arg: flag.GetArgs()) {
args.push_back(arg);
}
if (!ApplyFlag(pos, flag.GetName(), args, ctx, 0)) {
if (!ApplyFlag(pos, flag.GetName(), args, ctx)) {
return false;
}
}
Expand Down Expand Up @@ -260,23 +260,15 @@ namespace {
for (size_t i = 3; i < node->ChildrenSize(); ++i) {
if (node->Child(i)->IsCallable("EvaluateAtom")) {
hasPendingEvaluations = true;
break;
return res;
}
if (!EnsureAtom(*node->Child(i), ctx)) {
return {};
}
args.push_back(node->Child(i)->Content());
}

if (hasPendingEvaluations) {
if (!ValidateEvaluation(command, *node, ctx)) {
return {};
}

return res;
}

if (!ApplyFlag(ctx.GetPosition(node->Child(2)->Pos()), command, args, ctx, node->UniqueId())) {
if (!ApplyFlag(ctx.GetPosition(node->Child(2)->Pos()), command, args, ctx)) {
return {};
}

Expand Down Expand Up @@ -468,30 +460,7 @@ namespace {
return true;
}

bool ValidateEvaluation(const TStringBuf name, const TExprNode& node, TExprContext& ctx) {
if (name == "AddFileByUrl" || name == "AddFolderByUrl") {
if (node.ChildrenSize() < 4) {
ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder() << "Expected at least 4 arguments, but got " << node.ChildrenSize()));
return false;
}

if (node.Child(3)->IsCallable("EvaluateAtom")) {
return true;
}

if (!PendingEvaluationFiles.insert({TString(node.Child(3)->Content()), node.UniqueId()}).second) {
ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder() << "Detected evaluation cycle for file: " << node.Child(3)->Content()));
return false;
}

return true;
} else {
return true;
}
}

bool ApplyFlag(const TPosition& pos, const TStringBuf name, const TVector<TStringBuf>& args, TExprContext& ctx,
ui64 nodeUniqueId) {
bool ApplyFlag(const TPosition& pos, const TStringBuf name, const TVector<TStringBuf>& args, TExprContext& ctx) {
if (!IsSettingAllowed(pos, name, ctx)) {
return false;
}
Expand All @@ -505,15 +474,15 @@ namespace {
return false;
}
} else if (name == "AddFileByUrl") {
if (!AddFileByUrl(pos, args, ctx, nodeUniqueId)) {
if (!AddFileByUrl(pos, args, ctx)) {
return false;
}
} else if (name == "SetFileOption") {
if (!SetFileOption(pos, args, ctx)) {
return false;
}
} else if (name == "AddFolderByUrl") {
if (!AddFolderByUrl(pos, args, ctx, nodeUniqueId)) {
if (!AddFolderByUrl(pos, args, ctx)) {
return false;
}
} else if (name == "SetPackageVersion") {
Expand Down Expand Up @@ -1040,13 +1009,12 @@ namespace {
return true;
}

bool AddFileByUrl(const TPosition& pos, const TVector<TStringBuf>& args, TExprContext& ctx, ui64 nodeUniqueId) {
bool AddFileByUrl(const TPosition& pos, const TVector<TStringBuf>& args, TExprContext& ctx) {
if (args.size() < 2 || args.size() > 3) {
ctx.AddError(TIssue(pos, TStringBuilder() << "Expected 2 or 3 arguments, but got " << args.size()));
return false;
}

PendingEvaluationFiles.erase({TString(args[0]),nodeUniqueId});
TStringBuf token = args.size() == 3 ? args[2] : TStringBuf();
if (token) {
if (auto cred = Types.Credentials->FindCredential(token)) {
Expand Down Expand Up @@ -1160,13 +1128,12 @@ namespace {
return url;
}

bool AddFolderByUrl(const TPosition& pos, const TVector<TStringBuf>& args, TExprContext& ctx, ui64 nodeUniqueId) {
bool AddFolderByUrl(const TPosition& pos, const TVector<TStringBuf>& args, TExprContext& ctx) {
if (args.size() < 2 || args.size() > 3) {
ctx.AddError(TIssue(pos, TStringBuilder() << "Expected 2 or 3 arguments, but got " << args.size()));
return false;
}

PendingEvaluationFiles.erase({TString(args[0]),nodeUniqueId});
TStringBuf token = args.size() == 3 ? args[2] : TStringBuf();
if (token) {
if (auto cred = Types.Credentials->FindCredential(token)) {
Expand Down Expand Up @@ -1263,7 +1230,6 @@ namespace {
TString Username;
const TAllowSettingPolicy Policy;
TOperationStatistics Statistics;
THashSet<std::pair<TString, ui64>> PendingEvaluationFiles;
};
}

Expand Down
5 changes: 0 additions & 5 deletions ydb/library/yql/tests/sql/dq_file/part8/canondata/result.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@
}
],
"test.test[action-evaluate_pure--Results]": [],
"test.test[action-file_cycle--Results]": [
{
"uri": "file://test.test_action-file_cycle--Results_/extracted"
}
],
"test.test[action-nested_eval-default.txt-Analyze]": [
{
"checksum": "b4dd508a329723c74293d80f0278c705",
Expand Down
5 changes: 0 additions & 5 deletions ...ary/yql/tests/sql/dq_file/part8/canondata/test.test_action-file_cycle--Results_/extracted
View file

This file was deleted.

14 changes: 0 additions & 14 deletions ydb/library/yql/tests/sql/sql2yql/canondata/result.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,13 +426,6 @@
"uri": "https://{canondata_backend}/1936947/659b615f15086142a8960946dabd06b519d43335/resource.tar.gz#test_sql2yql.test_action-export_action_/sql.yql"
}
],
"test_sql2yql.test[action-file_cycle]": [
{
"checksum": "095c2c315ba26037554c056a02f5af05",
"size": 343,
"uri": "https://{canondata_backend}/1871002/4d084a25cea0c62bd09f3c9e9e7c2b56d112c5b1/resource.tar.gz#test_sql2yql.test_action-file_cycle_/sql.yql"
}
],
"test_sql2yql.test[action-inline_action]": [
{
"checksum": "928aaa85637625e18fef89b35501f0f3",
Expand Down Expand Up @@ -19004,13 +18997,6 @@
"uri": "https://{canondata_backend}/1871102/17992aa919577eec0f31ef167084ab70f41ccc80/resource.tar.gz#test_sql_format.test_action-export_action_/formatted.sql"
}
],
"test_sql_format.test[action-file_cycle]": [
{
"checksum": "5476df82092007027246993752e24f68",
"size": 45,
"uri": "https://{canondata_backend}/1871002/4d084a25cea0c62bd09f3c9e9e7c2b56d112c5b1/resource.tar.gz#test_sql_format.test_action-file_cycle_/formatted.sql"
}
],
"test_sql_format.test[action-inline_action]": [
{
"checksum": "7da554999ea3520183678c730ae2fffb",
Expand Down
2 changes: 0 additions & 2 deletions ydb/library/yql/tests/sql/suites/action/file_cycle.cfg
View file

This file was deleted.

3 changes: 0 additions & 3 deletions ydb/library/yql/tests/sql/suites/action/file_cycle.sql
View file

This file was deleted.

7 changes: 0 additions & 7 deletions ydb/library/yql/tests/sql/yt_native_file/part8/canondata/result.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@
"uri": "https://{canondata_backend}/1775059/105879cd875204bc7c66426c8bfdd2bbd01bfaf7/resource.tar.gz#test.test_action-evaluate_pure--Results_/results.txt"
}
],
"test.test[action-file_cycle--Debug]": [],
"test.test[action-file_cycle--Plan]": [],
"test.test[action-file_cycle--Results]": [
{
"uri": "file://test.test_action-file_cycle--Results_/extracted"
}
],
"test.test[action-nested_eval-default.txt-Debug]": [
{
"checksum": "dfc49c1f6f30ac41794475c26145cd89",
Expand Down
5 changes: 0 additions & 5 deletions .../tests/sql/yt_native_file/part8/canondata/test.test_action-file_cycle--Results_/extracted
View file

This file was deleted.

0 comments on commit 9529d2c

Please sign in to comment.