Browse Source

Added fatal type error handling to purecalc & peephole

init
commit_hash:b89977a75ce7119bfd34312b41e9382a28f13adc
vvvv 3 months ago
parent
commit
724d11e3a1

+ 0 - 43
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 ")

+ 0 - 1
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);

+ 46 - 0
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);
+    }
+}
+
 }

+ 2 - 0
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);
+
 }

+ 40 - 2
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);

+ 6 - 0
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) {

+ 2 - 0
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,

+ 12 - 2
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

+ 2 - 0
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);
+
 }

+ 3 - 1
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;

Some files were not shown because too many files changed in this diff