Added fatal type error handling to purecalc & peephole

init
commit_hash:b89977a75ce7119bfd34312b41e9382a28f13adc

14 files changed, 158 insertions, 61 deletions

diff --git a/yql/essentials/core/facade/yql_facade.cpp b/yql/essentials/core/facade/yql_facade.cpp
index 3b342a60e89..ee8e31eb354 100644
--- a/yql/essentials/core/facade/yql_facade.cpp
+++ b/yql/essentials/core/facade/yql_facade.cpp
@@ -1725,49 +1725,6 @@ TIssues TProgram::CompletedIssues() const {
     return result;
 }
 
-void TProgram::CheckFatalIssues(TIssues& issues) const {
-    bool isFatal = false;
-    auto checkIssue = [&](const TIssue& issue) {
-        if (issue.GetSeverity() == TSeverityIds::S_FATAL) {
-            isFatal = true;
-        }
-    };
-
-    std::function<void(const TIssuePtr& issue)> recursiveCheck = [&](const TIssuePtr& issue) {
-        if (isFatal) {
-            return;
-        }
-
-        checkIssue(*issue);
-        for (const auto& subissue : issue->GetSubIssues()) {
-            recursiveCheck(subissue);
-        }
-    };
-
-    for (const auto& issue : issues) {
-        if (isFatal) {
-            break;
-        }
-
-        checkIssue(issue);
-        // check subissues
-        for (const auto& subissue : issue.GetSubIssues()) {
-            recursiveCheck(subissue);
-        }
-    }
-
-    if (isFatal) {
-        TIssue result;
-        result.SetMessage(
-            TStringBuilder()
-                << "An abnormal situation found, so consider opening a bug report to YQL (st/YQLSUPPORT),"
-                << " because more detailed information is only available in server side logs and/or "
-                << "coredumps.");
-        result.SetCode(TIssuesIds::UNEXPECTED, TSeverityIds::S_FATAL);
-        issues.AddIssue(result);
-    }
-}
-
 TIssue MakeNoBlocksInfoIssue(const TVector<TString>& names, bool isTypes) {
     TIssue result;
     TString msg = TStringBuilder() << "Most frequent " << (isTypes ? "types " : "callables ")
diff --git a/yql/essentials/core/facade/yql_facade.h b/yql/essentials/core/facade/yql_facade.h
index eacbe1aa447..23aa2bd2b48 100644
--- a/yql/essentials/core/facade/yql_facade.h
+++ b/yql/essentials/core/facade/yql_facade.h
@@ -375,7 +375,6 @@ private:
 
 private:
     std::optional<bool> CheckFallbackIssues(const TIssues& issues);
-    void CheckFatalIssues(TIssues& issues) const;
     void HandleSourceCode(TString& sourceCode);
     void HandleTranslationSettings(NSQLTranslation::TTranslationSettings& loadedSettings,
         const NSQLTranslation::TTranslationSettings*& currentSettings);
diff --git a/yql/essentials/core/issue/yql_issue.cpp b/yql/essentials/core/issue/yql_issue.cpp
index 3d1ea9b9fe1..5fa6f02e69b 100644
--- a/yql/essentials/core/issue/yql_issue.cpp
+++ b/yql/essentials/core/issue/yql_issue.cpp
@@ -1,5 +1,7 @@
 #include "yql_issue.h"
 
+#include <util/string/builder.h>
+
 namespace NYql {
 
 const char IssueMapResource[] = "yql_issue.txt";
@@ -8,4 +10,48 @@ static_assert(DEFAULT_ERROR == TIssuesIds::DEFAULT_ERROR,
     "value of particular and common error mismatched for \"DEFAULT_ERROR\"");
 static_assert(UNEXPECTED_ERROR == TIssuesIds::UNEXPECTED,
     "value of particular and common error mismatched for \"UNEXPECTED_ERROR\"");
+
+void CheckFatalIssues(TIssues& issues) {
+    bool isFatal = false;
+    auto checkIssue = [&](const TIssue& issue) {
+        if (issue.GetSeverity() == TSeverityIds::S_FATAL) {
+            isFatal = true;
+        }
+    };
+
+    std::function<void(const TIssuePtr& issue)> recursiveCheck = [&](const TIssuePtr& issue) {
+        if (isFatal) {
+            return;
+        }
+
+        checkIssue(*issue);
+        for (const auto& subissue : issue->GetSubIssues()) {
+            recursiveCheck(subissue);
+        }
+    };
+
+    for (const auto& issue : issues) {
+        if (isFatal) {
+            break;
+        }
+
+        checkIssue(issue);
+        // check subissues
+        for (const auto& subissue : issue.GetSubIssues()) {
+            recursiveCheck(subissue);
+        }
+    }
+
+    if (isFatal) {
+        TIssue result;
+        result.SetMessage(
+            TStringBuilder()
+                << "An abnormal situation found, so consider opening a bug report to YQL (st/YQLSUPPORT),"
+                << " because more detailed information is only available in server side logs and/or "
+                << "coredumps.");
+        result.SetCode(TIssuesIds::UNEXPECTED, TSeverityIds::S_FATAL);
+        issues.AddIssue(result);
+    }
+}
+
 }
diff --git a/yql/essentials/core/issue/yql_issue.h b/yql/essentials/core/issue/yql_issue.h
index 8077f23c2f0..f4e6c63f8d4 100644
--- a/yql/essentials/core/issue/yql_issue.h
+++ b/yql/essentials/core/issue/yql_issue.h
@@ -48,4 +48,6 @@ inline TIssue YqlIssue(const TPosition& position, EYqlIssueCode id) {
     return YqlIssue(position, id, IssueCodeToString(id));
 }
 
+void CheckFatalIssues(TIssues& issues);
+
 }
diff --git a/yql/essentials/core/peephole_opt/yql_opt_peephole_physical.cpp b/yql/essentials/core/peephole_opt/yql_opt_peephole_physical.cpp
index 355762f7f5e..9efc34892d7 100644
--- a/yql/essentials/core/peephole_opt/yql_opt_peephole_physical.cpp
+++ b/yql/essentials/core/peephole_opt/yql_opt_peephole_physical.cpp
@@ -2305,14 +2305,52 @@ IGraphTransformer::TStatus PeepHoleBlockStage(const TExprNode::TPtr& input, TExp
     }, ctx, settings);
 }
 
+class TStrongTypeErrorProxy : public IGraphTransformer {
+public:
+    TStrongTypeErrorProxy(IGraphTransformer& inner)
+        : Inner_(inner)
+    {}
+
+    TStatus Transform(TExprNode::TPtr input, TExprNode::TPtr& output, TExprContext& ctx) final {
+        auto status = Inner_.Transform(input, output, ctx);
+        CheckFatalTypeError(status);
+        return status;
+    }
+
+    NThreading::TFuture<void> GetAsyncFuture(const TExprNode& input) final {
+        return Inner_.GetAsyncFuture(input);
+    }
+
+    TStatus ApplyAsyncChanges(TExprNode::TPtr input, TExprNode::TPtr& output, TExprContext& ctx) final {
+        auto status = Inner_.ApplyAsyncChanges(input, output, ctx);
+        CheckFatalTypeError(status);
+        return status;
+    }
+
+    void Rewind() final {
+        return Inner_.Rewind();
+    }
+
+    TStatistics GetStatistics() const final {
+        return Inner_.GetStatistics();
+    }
+
+private:
+    IGraphTransformer& Inner_;
+};
+
+TAutoPtr<IGraphTransformer> MakeStrongTypeErrorProxy(IGraphTransformer& inner) {
+    return new TStrongTypeErrorProxy(inner);
+}
+
 template<bool FinalStage>
 void AddStandardTransformers(TTransformationPipeline& pipelene, IGraphTransformer* typeAnnotator) {
     auto issueCode = TIssuesIds::CORE_EXEC;
     pipelene.AddServiceTransformers(issueCode);
     if (typeAnnotator) {
-        pipelene.Add(*typeAnnotator, "TypeAnnotation", issueCode);
+        pipelene.Add(MakeStrongTypeErrorProxy(*typeAnnotator), "TypeAnnotation", issueCode);
     } else {
-        pipelene.AddTypeAnnotationTransformer(issueCode);
+        pipelene.AddTypeAnnotationTransformerWithMode(issueCode, ETypeCheckMode::Repeat);
     }
 
     pipelene.AddPostTypeAnnotation(true, FinalStage, issueCode);
diff --git a/yql/essentials/core/services/yql_transform_pipeline.cpp b/yql/essentials/core/services/yql_transform_pipeline.cpp
index 427801fc6b2..9154a76d69f 100644
--- a/yql/essentials/core/services/yql_transform_pipeline.cpp
+++ b/yql/essentials/core/services/yql_transform_pipeline.cpp
@@ -259,6 +259,12 @@ TTransformationPipeline& TTransformationPipeline::AddTypeAnnotationTransformer(
     return *this;
 }
 
+TTransformationPipeline& TTransformationPipeline::AddTypeAnnotationTransformerWithMode(EYqlIssueCode issueCode, ETypeCheckMode mode) {
+    auto callableTransformer = CreateExtCallableTypeAnnotationTransformer(*TypeAnnotationContext_);
+    AddTypeAnnotationTransformer(callableTransformer, issueCode, mode);
+    return *this;
+}
+
 TTransformationPipeline& TTransformationPipeline::AddTypeAnnotationTransformer(EYqlIssueCode issueCode, bool twoStages)
 {
     if (twoStages) {
diff --git a/yql/essentials/core/services/yql_transform_pipeline.h b/yql/essentials/core/services/yql_transform_pipeline.h
index d21290e2a53..e9326746faf 100644
--- a/yql/essentials/core/services/yql_transform_pipeline.h
+++ b/yql/essentials/core/services/yql_transform_pipeline.h
@@ -43,6 +43,8 @@ public:
     TTransformationPipeline& AddTableMetadataLoaderTransformer(EYqlIssueCode issueCode = TIssuesIds::CORE_TABLE_METADATA_LOADER);
     TTransformationPipeline& AddTypeAnnotationTransformer(TAutoPtr<IGraphTransformer> callableTransformer, EYqlIssueCode issueCode = TIssuesIds::CORE_TYPE_ANN,
         ETypeCheckMode mode = ETypeCheckMode::Single);
+    TTransformationPipeline& AddTypeAnnotationTransformerWithMode(EYqlIssueCode issueCode = TIssuesIds::CORE_TYPE_ANN,
+        ETypeCheckMode mode = ETypeCheckMode::Single);
     TTransformationPipeline& AddTypeAnnotationTransformer(EYqlIssueCode issueCode = TIssuesIds::CORE_TYPE_ANN, bool twoStages = false);
 
     TTransformationPipeline& Add(TAutoPtr<IGraphTransformer> transformer, const TString& stageName,
diff --git a/yql/essentials/core/type_ann/type_ann_expr.cpp b/yql/essentials/core/type_ann/type_ann_expr.cpp
index 8b213a0431a..4ac30a9d8aa 100644
--- a/yql/essentials/core/type_ann/type_ann_expr.cpp
+++ b/yql/essentials/core/type_ann/type_ann_expr.cpp
@@ -70,8 +70,8 @@ public:
             IsComplete = true;
         }
 
-        if (Mode == ETypeCheckMode::Repeat && status == TStatus::Error) {
-            throw yexception() << "Detected a type error after initial validation";
+        if (Mode == ETypeCheckMode::Repeat) {
+            CheckFatalTypeError(status);
         }
 
         return status;
@@ -110,6 +110,10 @@ public:
             Processed.clear();
         }
 
+        if (Mode == ETypeCheckMode::Repeat) {
+            CheckFatalTypeError(combinedStatus);
+        }
+
         return combinedStatus;
     }
 
@@ -712,4 +716,10 @@ TExprNode::TPtr ParseAndAnnotate(
     return exprRoot;
 }
 
+void CheckFatalTypeError(IGraphTransformer::TStatus status) {
+    if (status == IGraphTransformer::TStatus::Error) {
+        throw yexception() << "Detected a type error after initial validation";
+    }
+}
+
 } // namespace NYql
diff --git a/yql/essentials/core/type_ann/type_ann_expr.h b/yql/essentials/core/type_ann/type_ann_expr.h
index 5b9c072a610..4cbb12a881a 100644
--- a/yql/essentials/core/type_ann/type_ann_expr.h
+++ b/yql/essentials/core/type_ann/type_ann_expr.h
@@ -33,4 +33,6 @@ TExprNode::TPtr ParseAndAnnotate(
         TExprContext& exprCtx, bool instant, bool wholeProgram,
         TTypeAnnotationContext& typeAnnotationContext);
 
+void CheckFatalTypeError(IGraphTransformer::TStatus status);
+
 }
diff --git a/yql/essentials/public/purecalc/common/program_factory.cpp b/yql/essentials/public/purecalc/common/program_factory.cpp
index 8452dc3d003..ec4cbe2a87f 100644
--- a/yql/essentials/public/purecalc/common/program_factory.cpp
+++ b/yql/essentials/public/purecalc/common/program_factory.cpp
@@ -27,7 +27,9 @@ TProgramFactory::TProgramFactory(const TProgramFactoryOptions& options)
     UserData_ = GetYqlModuleResolver(ExprContext_, ModuleResolver_, Options_.UserData_, {}, {});
 
     if (!ModuleResolver_) {
-        ythrow TCompileError("", ExprContext_.IssueManager.GetIssues().ToString()) << "failed to compile modules";
+        auto issues = ExprContext_.IssueManager.GetIssues();
+        CheckFatalIssues(issues);
+        ythrow TCompileError("", issues.ToString()) << "failed to compile modules";
     }
 
     TVector<TString> UDFsPaths;
diff --git a/yql/essentials/public/purecalc/common/type_from_schema.cpp b/yql/essentials/public/purecalc/common/type_from_schema.cpp
index 373283a1a8e..dafb4b29768 100644
--- a/yql/essentials/public/purecalc/common/type_from_schema.cpp
+++ b/yql/essentials/public/purecalc/common/type_from_schema.cpp
@@ -210,7 +210,9 @@ namespace NYql::NPureCalc {
         const auto* type = NCommon::ParseTypeFromYson(yson, ctx);
 
         if (!type) {
-            ythrow TCompileError("", ctx.IssueManager.GetIssues().ToString())
+            auto issues = ctx.IssueManager.GetIssues();
+            CheckFatalIssues(issues);
+            ythrow TCompileError("", issues.ToString())
                 << "Incorrect schema: " << NYT::NodeToYsonString(yson, NYson::EYsonFormat::Text);
         }
 
@@ -232,7 +234,9 @@ namespace NYql::NPureCalc {
         auto result = ctx.MakeType<TStructExprType>(items);
 
         if (!result->Validate(TPosition(), ctx)) {
-            ythrow TCompileError("", ctx.IssueManager.GetIssues().ToString()) << "Incorrect extended struct type";
+            auto issues = ctx.IssueManager.GetIssues();
+            CheckFatalIssues(issues);
+            ythrow TCompileError("", issues.ToString()) << "Incorrect extended struct type";
         }
 
         return result;
diff --git a/yql/essentials/public/purecalc/common/worker_factory.cpp b/yql/essentials/public/purecalc/common/worker_factory.cpp
index 27ac0acda8b..c73f38afb42 100644
--- a/yql/essentials/public/purecalc/common/worker_factory.cpp
+++ b/yql/essentials/public/purecalc/common/worker_factory.cpp
@@ -61,7 +61,7 @@ TWorkerFactory<TBase>::TWorkerFactory(TWorkerFactoryOptions options, EProcessorM
     for (ui32 i = 0; i < inputsCount; ++i) {
         const auto* originalInputType = MakeTypeFromSchema(inputSchemas[i], ExprContext_);
         if (!ValidateInputSchema(originalInputType, ExprContext_)) {
-            ythrow TCompileError("", ExprContext_.IssueManager.GetIssues().ToString()) << "invalid schema for #" << i << " input";
+            ythrow TCompileError("", GetIssues().ToString()) << "invalid schema for #" << i << " input";
         }
 
         const auto* originalStructType = originalInputType->template Cast<TStructExprType>();
@@ -76,7 +76,7 @@ TWorkerFactory<TBase>::TWorkerFactory(TWorkerFactoryOptions options, EProcessorM
             columnsSet.insert(TString(structItem->GetName()));
 
             if (!UseSystemColumns_ && structItem->GetName().StartsWith(PurecalcSysColumnsPrefix)) {
-                ythrow TCompileError("", ExprContext_.IssueManager.GetIssues().ToString())
+                ythrow TCompileError("", GetIssues().ToString())
                     << "#" << i << " input provides system column " << structItem->GetName()
                     << ", but it is forbidden by options";
             }
@@ -89,7 +89,7 @@ TWorkerFactory<TBase>::TWorkerFactory(TWorkerFactoryOptions options, EProcessorM
     if (!outputSchema.IsNull()) {
         OutputType_ = MakeTypeFromSchema(outputSchema, ExprContext_);
         if (!ValidateOutputSchema(OutputType_, ExprContext_)) {
-            ythrow TCompileError("", ExprContext_.IssueManager.GetIssues().ToString()) << "invalid output schema";
+            ythrow TCompileError("", GetIssues().ToString()) << "invalid output schema";
         }
     } else {
         OutputType_ = nullptr;
@@ -121,7 +121,7 @@ TWorkerFactory<TBase>::TWorkerFactory(TWorkerFactoryOptions options, EProcessorM
             }
         }
         if (!OutputType_) {
-            ythrow TCompileError("", ExprContext_.IssueManager.GetIssues().ToString()) << "cannot deduce output schema";
+            ythrow TCompileError("", GetIssues().ToString()) << "cannot deduce output schema";
         }
     }
 }
@@ -208,12 +208,11 @@ TExprNode::TPtr TWorkerFactory<TBase>::Compile(
         astRes = ParseAst(TString(query));
     }
 
+    ExprContext_.IssueManager.AddIssues(astRes.Issues);
     if (!astRes.IsOk()) {
-        ythrow TCompileError(TString(query), astRes.Issues.ToString()) << "failed to parse " << mode;
+        ythrow TCompileError(TString(query), GetIssues().ToString()) << "failed to parse " << mode;
     }
 
-    ExprContext_.IssueManager.AddIssues(astRes.Issues);
-
     if (ETraceLevel::TRACE_DETAIL <= StdDbgLevel()) {
         Cdbg << "Before optimization:" << Endl;
         astRes.Root->PrettyPrintTo(Cdbg, TAstPrintFlags::PerLine | TAstPrintFlags::ShortQuote | TAstPrintFlags::AdaptArbitraryContent);
@@ -225,7 +224,7 @@ TExprNode::TPtr TWorkerFactory<TBase>::Compile(
     if (!CompileExpr(*astRes.Root, exprRoot, ExprContext_, moduleResolver.get(), nullptr, 0, syntaxVersion)) {
         TStringStream astStr;
         astRes.Root->PrettyPrintTo(astStr, TAstPrintFlags::ShortQuote | TAstPrintFlags::PerLine);
-        ythrow TCompileError(astStr.Str(), ExprContext_.IssueManager.GetIssues().ToString()) << "failed to compile";
+        ythrow TCompileError(astStr.Str(), GetIssues().ToString()) << "failed to compile";
     }
 
 
@@ -343,7 +342,7 @@ TExprNode::TPtr TWorkerFactory<TBase>::Compile(
     NCommon::TransformerStatsToYson("", transformStats, writer);
     YQL_CLOG(DEBUG, Core) << "Transform stats: " << out.Str();
     if (status == IGraphTransformer::TStatus::Error) {
-        ythrow TCompileError("", ExprContext_.IssueManager.GetIssues().ToString()) << "Failed to optimize";
+        ythrow TCompileError("", GetIssues().ToString()) << "Failed to optimize";
     }
 
     IOutputStream* exprOut = nullptr;
@@ -463,7 +462,9 @@ const THashSet<TString>& TWorkerFactory<TBase>::GetUsedColumns() const {
 
 template <typename TBase>
 TIssues TWorkerFactory<TBase>::GetIssues() const {
-    return ExprContext_.IssueManager.GetCompletedIssues();
+    auto issues = ExprContext_.IssueManager.GetCompletedIssues();
+    CheckFatalIssues(issues);
+    return issues;
 }
 
 template <typename TBase>
diff --git a/yql/essentials/public/purecalc/ut/test_fatal_err.cpp b/yql/essentials/public/purecalc/ut/test_fatal_err.cpp
new file mode 100644
index 00000000000..bb1452b16e3
--- /dev/null
+++ b/yql/essentials/public/purecalc/ut/test_fatal_err.cpp
@@ -0,0 +1,27 @@
+#include <yql/essentials/public/purecalc/purecalc.h>
+#include <yql/essentials/public/purecalc/io_specs/protobuf/spec.h>
+#include <yql/essentials/public/purecalc/ut/protos/test_structs.pb.h>
+#include <yql/essentials/public/purecalc/ut/empty_stream.h>
+
+#include <library/cpp/testing/unittest/registar.h>
+
+Y_UNIT_TEST_SUITE(TestFatalError) {
+    Y_UNIT_TEST(TestFailType) {
+        using namespace NYql::NPureCalc;
+
+        auto options = TProgramFactoryOptions();
+        auto factory = MakeProgramFactory(options);
+
+        try {
+            factory->MakePullListProgram(
+                TProtobufInputSpec<NPureCalcProto::TStringMessage>(),
+                TProtobufOutputSpec<NPureCalcProto::TStringMessage>(),
+                "pragma warning(\"disable\",\"4510\");select unwrap(cast(Yql::FailMe(AsAtom('type')) as Utf8)) as X;",
+                ETranslationMode::SQL
+            );
+            UNIT_FAIL("Exception is expected");
+        } catch (const TCompileError& e) {
+            UNIT_ASSERT_C(e.GetIssues().Contains("abnormal"), e.GetIssues());
+        }
+    }
+}
diff --git a/yql/essentials/public/purecalc/ut/ya.make b/yql/essentials/public/purecalc/ut/ya.make
index 474280a8e27..701d0cb37e2 100644
--- a/yql/essentials/public/purecalc/ut/ya.make
+++ b/yql/essentials/public/purecalc/ut/ya.make
@@ -11,6 +11,7 @@ SRCS(
     test_udf.cpp
     test_user_data.cpp
     test_eval.cpp
+    test_fatal_err.cpp
     test_pool.cpp
     test_mixed_allocators.cpp
 )