Skip to content

Commit addde5a

Browse files
committed
Revert "YQL-17476 check for cycle during evaluation of file args (ydb-platform#1608)"
This reverts commit 1c12b54.
1 parent df2881b commit addde5a

File tree

8 files changed

+8
-83
lines changed

8 files changed

+8
-83
lines changed

ydb/library/yql/providers/config/yql_config_provider.cpp

+8-42
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ namespace {
168168
for (auto& arg: flag.GetArgs()) {
169169
args.push_back(arg);
170170
}
171-
if (!ApplyFlag(pos, flag.GetName(), args, ctx, 0)) {
171+
if (!ApplyFlag(pos, flag.GetName(), args, ctx)) {
172172
return false;
173173
}
174174
}
@@ -260,23 +260,15 @@ namespace {
260260
for (size_t i = 3; i < node->ChildrenSize(); ++i) {
261261
if (node->Child(i)->IsCallable("EvaluateAtom")) {
262262
hasPendingEvaluations = true;
263-
break;
263+
return res;
264264
}
265265
if (!EnsureAtom(*node->Child(i), ctx)) {
266266
return {};
267267
}
268268
args.push_back(node->Child(i)->Content());
269269
}
270270

271-
if (hasPendingEvaluations) {
272-
if (!ValidateEvaluation(command, *node, ctx)) {
273-
return {};
274-
}
275-
276-
return res;
277-
}
278-
279-
if (!ApplyFlag(ctx.GetPosition(node->Child(2)->Pos()), command, args, ctx, node->UniqueId())) {
271+
if (!ApplyFlag(ctx.GetPosition(node->Child(2)->Pos()), command, args, ctx)) {
280272
return {};
281273
}
282274

@@ -468,30 +460,7 @@ namespace {
468460
return true;
469461
}
470462

471-
bool ValidateEvaluation(const TStringBuf name, const TExprNode& node, TExprContext& ctx) {
472-
if (name == "AddFileByUrl" || name == "AddFolderByUrl") {
473-
if (node.ChildrenSize() < 4) {
474-
ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder() << "Expected at least 4 arguments, but got " << node.ChildrenSize()));
475-
return false;
476-
}
477-
478-
if (node.Child(3)->IsCallable("EvaluateAtom")) {
479-
return true;
480-
}
481-
482-
if (!PendingEvaluationFiles.insert({TString(node.Child(3)->Content()), node.UniqueId()}).second) {
483-
ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder() << "Detected evaluation cycle for file: " << node.Child(3)->Content()));
484-
return false;
485-
}
486-
487-
return true;
488-
} else {
489-
return true;
490-
}
491-
}
492-
493-
bool ApplyFlag(const TPosition& pos, const TStringBuf name, const TVector<TStringBuf>& args, TExprContext& ctx,
494-
ui64 nodeUniqueId) {
463+
bool ApplyFlag(const TPosition& pos, const TStringBuf name, const TVector<TStringBuf>& args, TExprContext& ctx) {
495464
if (!IsSettingAllowed(pos, name, ctx)) {
496465
return false;
497466
}
@@ -505,15 +474,15 @@ namespace {
505474
return false;
506475
}
507476
} else if (name == "AddFileByUrl") {
508-
if (!AddFileByUrl(pos, args, ctx, nodeUniqueId)) {
477+
if (!AddFileByUrl(pos, args, ctx)) {
509478
return false;
510479
}
511480
} else if (name == "SetFileOption") {
512481
if (!SetFileOption(pos, args, ctx)) {
513482
return false;
514483
}
515484
} else if (name == "AddFolderByUrl") {
516-
if (!AddFolderByUrl(pos, args, ctx, nodeUniqueId)) {
485+
if (!AddFolderByUrl(pos, args, ctx)) {
517486
return false;
518487
}
519488
} else if (name == "SetPackageVersion") {
@@ -1026,13 +995,12 @@ namespace {
1026995
return true;
1027996
}
1028997

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

1035-
PendingEvaluationFiles.erase({TString(args[0]),nodeUniqueId});
10361004
TStringBuf token = args.size() == 3 ? args[2] : TStringBuf();
10371005
if (token) {
10381006
if (auto cred = Types.Credentials->FindCredential(token)) {
@@ -1146,13 +1114,12 @@ namespace {
11461114
return url;
11471115
}
11481116

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

1155-
PendingEvaluationFiles.erase({TString(args[0]),nodeUniqueId});
11561123
TStringBuf token = args.size() == 3 ? args[2] : TStringBuf();
11571124
if (token) {
11581125
if (auto cred = Types.Credentials->FindCredential(token)) {
@@ -1249,7 +1216,6 @@ namespace {
12491216
TString Username;
12501217
const TAllowSettingPolicy Policy;
12511218
TOperationStatistics Statistics;
1252-
THashSet<std::pair<TString, ui64>> PendingEvaluationFiles;
12531219
};
12541220
}
12551221

ydb/library/yql/tests/sql/dq_file/part8/canondata/result.json

-5
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,6 @@
2121
}
2222
],
2323
"test.test[action-evaluate_pure--Results]": [],
24-
"test.test[action-file_cycle--Results]": [
25-
{
26-
"uri": "file://test.test_action-file_cycle--Results_/extracted"
27-
}
28-
],
2924
"test.test[action-nested_eval-default.txt-Analyze]": [
3025
{
3126
"checksum": "b4dd508a329723c74293d80f0278c705",

ydb/library/yql/tests/sql/dq_file/part8/canondata/test.test_action-file_cycle--Results_/extracted

-5
This file was deleted.

ydb/library/yql/tests/sql/sql2yql/canondata/result.json

-14
Original file line numberDiff line numberDiff line change
@@ -412,13 +412,6 @@
412412
"uri": "https://{canondata_backend}/1936947/659b615f15086142a8960946dabd06b519d43335/resource.tar.gz#test_sql2yql.test_action-export_action_/sql.yql"
413413
}
414414
],
415-
"test_sql2yql.test[action-file_cycle]": [
416-
{
417-
"checksum": "095c2c315ba26037554c056a02f5af05",
418-
"size": 343,
419-
"uri": "https://{canondata_backend}/1871002/4d084a25cea0c62bd09f3c9e9e7c2b56d112c5b1/resource.tar.gz#test_sql2yql.test_action-file_cycle_/sql.yql"
420-
}
421-
],
422415
"test_sql2yql.test[action-inline_action]": [
423416
{
424417
"checksum": "8a5ec6d478e5d8d125841b8105d50706",
@@ -18185,13 +18178,6 @@
1818518178
"uri": "https://{canondata_backend}/1871102/17992aa919577eec0f31ef167084ab70f41ccc80/resource.tar.gz#test_sql_format.test_action-export_action_/formatted.sql"
1818618179
}
1818718180
],
18188-
"test_sql_format.test[action-file_cycle]": [
18189-
{
18190-
"checksum": "5476df82092007027246993752e24f68",
18191-
"size": 45,
18192-
"uri": "https://{canondata_backend}/1871002/4d084a25cea0c62bd09f3c9e9e7c2b56d112c5b1/resource.tar.gz#test_sql_format.test_action-file_cycle_/formatted.sql"
18193-
}
18194-
],
1819518181
"test_sql_format.test[action-inline_action]": [
1819618182
{
1819718183
"checksum": "7da554999ea3520183678c730ae2fffb",

ydb/library/yql/tests/sql/suites/action/file_cycle.cfg

-2
This file was deleted.

ydb/library/yql/tests/sql/suites/action/file_cycle.sql

-3
This file was deleted.

ydb/library/yql/tests/sql/yt_native_file/part8/canondata/result.json

-7
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@
2020
"uri": "https://{canondata_backend}/1900335/4871e48d29e4933514d3ed0c9c5d19de571eda1f/resource.tar.gz#test.test_action-evaluate_pure--Results_/results.txt"
2121
}
2222
],
23-
"test.test[action-file_cycle--Debug]": [],
24-
"test.test[action-file_cycle--Plan]": [],
25-
"test.test[action-file_cycle--Results]": [
26-
{
27-
"uri": "file://test.test_action-file_cycle--Results_/extracted"
28-
}
29-
],
3023
"test.test[action-nested_eval-default.txt-Debug]": [
3124
{
3225
"checksum": "79af7d3cdd20c4123718aae10c3202d7",

ydb/library/yql/tests/sql/yt_native_file/part8/canondata/test.test_action-file_cycle--Results_/extracted

-5
This file was deleted.

0 commit comments

Comments
 (0)