fix: set output to id node for equivalent agg calls

4paradigm · Feb 11, 2025 · 7ea359f · 7ea359f
1 parent 522dbe9
commit 7ea359f
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 7 deletions.
diff --git a/hybridse/include/node/sql_node.h b/hybridse/include/node/sql_node.h
@@ -17,6 +17,7 @@
 #ifndef HYBRIDSE_INCLUDE_NODE_SQL_NODE_H_
 #define HYBRIDSE_INCLUDE_NODE_SQL_NODE_H_
 
+#include <absl/status/status.h>
 #include <iostream>
 #include <map>
 #include <memory>
@@ -30,6 +31,7 @@
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
+#include "base/fe_status.h"
 #include "boost/algorithm/string.hpp"
 #include "boost/algorithm/string/predicate.hpp"
 #include "boost/filesystem/operations.hpp"
@@ -1878,21 +1880,30 @@ class LetExpr : public ExprNode {
      public:
         LetCtxEntry(ExprIdNode *id_node, ExprNode *expr, const FrameNode *frame)
             : id_node(id_node), expr(expr), frame(frame) {}
+
         ExprIdNode *id_node;
         ExprNode *expr;  // referred udaf call
         const FrameNode *frame;
     };
 
     class LetContext {
      public:
-        void Append(ExprIdNode *k, ExprNode *v, const FrameNode* frame) {
-            if (cache.find(k) == cache.end()) {
+        base::Status Append(ExprIdNode *k, ExprNode *v, const FrameNode *frame) {
+            auto it = cache.find(k);
+            if (it == cache.end()) {
                 bindings.emplace_back(k, v, frame);
                 cache.emplace(k, v);
+            } else {
+                CHECK_TRUE(v == it->second, common::kPlanError,
+                           "let context: try mapping id node to two different resolved nodes");
             }
+
+            return base::Status::OK();
         }
 
         bool empty() const noexcept { return bindings.empty(); }
+
+        // necessary let bindings, not all cached entry will appear in bindings
         std::vector<LetCtxEntry> bindings;
         absl::flat_hash_map<ExprIdNode *, ExprNode *> cache;
     };

diff --git a/hybridse/src/passes/lambdafy_projects.cc b/hybridse/src/passes/lambdafy_projects.cc
@@ -110,7 +110,7 @@ Status LambdafyProjects::VisitExpr(const node::ExprNode* ori_expr,
         *has_agg = it->second.has_agg;
 
         // memories in LetContext
-        let_ctx.Append(it->second.id_node, it->second.lambdafied, frame);
+        CHECK_STATUS(let_ctx.Append(it->second.id_node, it->second.lambdafied, frame));
 
         return base::Status::OK();
     }
@@ -142,7 +142,8 @@ Status LambdafyProjects::VisitExpr(const node::ExprNode* ori_expr,
                                "nested udaf call should inferred before")
                     auto it = cache.find(ori_expr);
                     CHECK_TRUE(it != cache.end() , kCodegenError, "no expr id node found for agg call");
-                    let_ctx.Append(it->second.id_node, *out, frame);
+                    CHECK_STATUS(let_ctx.Append(it->second.id_node, *out, frame));
+                    *out = it->second.id_node;
                 }
                 return base::Status::OK();
             }
@@ -215,7 +216,8 @@ Status LambdafyProjects::VisitExpr(const node::ExprNode* ori_expr,
 
         if (inside_agg_ctx) {
             // the agg call is inside another agg, just memories in let context
-            let_ctx.Append(nested_agg_id_node, expr_copy, frame);
+            CHECK_STATUS(let_ctx.Append(nested_agg_id_node, expr_copy, frame));
+            expr_copy = nested_agg_id_node;
         }
     }
     *out = expr_copy;

diff --git a/hybridse/src/passes/lambdafy_projects.h b/hybridse/src/passes/lambdafy_projects.h
@@ -19,7 +19,6 @@
 
 #include <string>
 #include <unordered_set>
-#include <utility>
 #include <vector>
 
 #include "absl/container/flat_hash_map.h"
@@ -64,7 +63,6 @@ class LambdafyProjects {
 
         node::ExprNode* lambdafied;
         bool has_agg;
-        // lambdafied udaf expr nodes may have corresponding id not, if it is somewhere used as nested udaf call
         node::ExprIdNode* id_node;
     };
     // expr node -> (lambdafied expr node, has aggregate, is aggregate call itself)