Merge pull request #2941 from kuzudb/fix-uuid-bug

Fix generate_random_uuid bug
kuzudb · Feb 25, 2024 · 1ce0728 · 1ce0728
2 parents f2619d5 + 53e0d25
commit 1ce0728
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 29 deletions.
diff --git a/src/binder/expression_visitor.cpp b/src/binder/expression_visitor.cpp
@@ -1,10 +1,12 @@
 #include "binder/expression_visitor.h"
 
 #include "binder/expression/case_expression.h"
+#include "binder/expression/function_expression.h"
 #include "binder/expression/node_expression.h"
 #include "binder/expression/property_expression.h"
 #include "binder/expression/rel_expression.h"
 #include "binder/expression/subquery_expression.h"
+#include "common/cast.h"
 
 using namespace kuzu::common;
 
@@ -100,6 +102,22 @@ bool ExpressionVisitor::isConstant(const Expression& expression) {
     return true;
 }
 
+bool ExpressionVisitor::isRandom(const Expression& expression) {
+    if (expression.expressionType != ExpressionType::FUNCTION) {
+        return false;
+    }
+    auto& funcExpr = ku_dynamic_cast<const Expression&, const FunctionExpression&>(expression);
+    if (funcExpr.getFunctionName() == GEN_RANDOM_UUID_FUNC_NAME) {
+        return true;
+    }
+    for (auto& child : ExpressionChildrenCollector::collectChildren(expression)) {
+        if (isRandom(*child)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 bool ExpressionVisitor::satisfyAny(
     const Expression& expression, const std::function<bool(const Expression&)>& condition) {
     if (condition(expression)) {

diff --git a/src/include/binder/expression_visitor.h b/src/include/binder/expression_visitor.h
@@ -41,6 +41,8 @@ class ExpressionVisitor {
 
     static bool isConstant(const Expression& expression);
 
+    static bool isRandom(const Expression& expression);
+
 private:
     static bool satisfyAny(
         const Expression& expression, const std::function<bool(const Expression&)>& condition);

diff --git a/src/include/function/pointer_function_executor.h b/src/include/function/pointer_function_executor.h
@@ -3,17 +3,21 @@
 #include "common/vector/value_vector.h"
 
 namespace kuzu {
-namespace main {
-class ClientContext;
-}
-
 namespace function {
 
 struct PointerFunctionExecutor {
-
     template<typename RESULT_TYPE, typename OP>
     static void execute(common::ValueVector& result, void* dataPtr) {
-        OP::operation(result, dataPtr);
+        if (result.state->selVector->isUnfiltered()) {
+            for (auto i = 0u; i < result.state->selVector->selectedSize; i++) {
+                OP::operation(result.getValue<RESULT_TYPE>(i), dataPtr);
+            }
+        } else {
+            for (auto i = 0u; i < result.state->selVector->selectedSize; i++) {
+                auto pos = result.state->selVector->selectedPositions[i];
+                OP::operation(result.getValue<RESULT_TYPE>(pos), dataPtr);
+            }
+        }
     }
 };
 

diff --git a/src/include/function/uuid/functions/gen_random_uuid.h b/src/include/function/uuid/functions/gen_random_uuid.h
@@ -4,23 +4,11 @@
 #include "main/client_context.h"
 
 namespace kuzu {
-namespace main {
-class ClientContext;
-}
-
-namespace common {
-struct ku_uuid_t;
-} // namespace common
-
 namespace function {
 
 struct GenRandomUUID {
-    static inline void operation(common::ValueVector& result, void* dataPtr) {
-        KU_ASSERT(result.state->isFlat());
-        auto resultValues = (common::ku_uuid_t*)result.getData();
-        auto idx = result.state->selVector->selectedPositions[0];
-        KU_ASSERT(idx == 0);
-        resultValues[idx] = common::UUID::generateRandomUUID(
+    static void operation(common::ku_uuid_t& input, void* dataPtr) {
+        input = common::UUID::generateRandomUUID(
             reinterpret_cast<main::ClientContext*>(dataPtr)->getRandomEngine());
     }
 };

diff --git a/src/optimizer/factorization_rewriter.cpp b/src/optimizer/factorization_rewriter.cpp
@@ -1,5 +1,6 @@
 #include "optimizer/factorization_rewriter.h"
 
+#include "binder/expression_visitor.h"
 #include "planner/operator/extend/logical_extend.h"
 #include "planner/operator/extend/logical_recursive_extend.h"
 #include "planner/operator/factorization/flatten_resolver.h"
@@ -74,11 +75,26 @@ void FactorizationRewriter::visitIntersect(planner::LogicalOperator* op) {
 
 void FactorizationRewriter::visitProjection(planner::LogicalOperator* op) {
     auto projection = (LogicalProjection*)op;
-    for (auto& expression : projection->getExpressionsToProject()) {
-        auto dependentGroupsPos = op->getChild(0)->getSchema()->getDependentGroupsPos(expression);
-        auto groupsPosToFlatten = factorization::FlattenAllButOne::getGroupsPosToFlatten(
-            dependentGroupsPos, op->getChild(0)->getSchema());
+    bool hasRandomFunction = false;
+    for (auto& expr : projection->getExpressionsToProject()) {
+        if (ExpressionVisitor::isRandom(*expr)) {
+            hasRandomFunction = true;
+        }
+    }
+    if (hasRandomFunction) {
+        // Fall back to tuple-at-a-time evaluation.
+        auto groupsPos = op->getChild(0)->getSchema()->getGroupsPosInScope();
+        auto groupsPosToFlatten = factorization::FlattenAll::getGroupsPosToFlatten(
+            groupsPos, op->getChild(0)->getSchema());
         projection->setChild(0, appendFlattens(projection->getChild(0), groupsPosToFlatten));
+    } else {
+        for (auto& expression : projection->getExpressionsToProject()) {
+            auto dependentGroupsPos =
+                op->getChild(0)->getSchema()->getDependentGroupsPos(expression);
+            auto groupsPosToFlatten = factorization::FlattenAllButOne::getGroupsPosToFlatten(
+                dependentGroupsPos, op->getChild(0)->getSchema());
+            projection->setChild(0, appendFlattens(projection->getChild(0), groupsPosToFlatten));
+        }
     }
 }
 

diff --git a/src/planner/plan/append_projection.cpp b/src/planner/plan/append_projection.cpp
@@ -1,3 +1,4 @@
+#include "binder/expression_visitor.h"
 #include "planner/operator/factorization/flatten_resolver.h"
 #include "planner/operator/logical_projection.h"
 #include "planner/planner.h"
@@ -11,11 +12,23 @@ void Planner::appendProjection(const expression_vector& expressionsToProject, Lo
     for (auto& expression : expressionsToProject) {
         planSubqueryIfNecessary(expression, plan);
     }
-    for (auto& expression : expressionsToProject) {
-        auto dependentGroupsPos = plan.getSchema()->getDependentGroupsPos(expression);
-        auto groupsPosToFlatten = factorization::FlattenAllButOne::getGroupsPosToFlatten(
-            dependentGroupsPos, plan.getSchema());
-        appendFlattens(groupsPosToFlatten, plan);
+    bool hasRandomFunction = false;
+    for (auto& expr : expressionsToProject) {
+        if (ExpressionVisitor::isRandom(*expr)) {
+            hasRandomFunction = true;
+        }
+    }
+    if (hasRandomFunction) {
+        // Fall back to tuple-at-a-time evaluation.
+        appendMultiplicityReducer(plan);
+        appendFlattens(plan.getSchema()->getGroupsPosInScope(), plan);
+    } else {
+        for (auto& expression : expressionsToProject) {
+            auto dependentGroupsPos = plan.getSchema()->getDependentGroupsPos(expression);
+            auto groupsPosToFlatten = factorization::FlattenAllButOne::getGroupsPosToFlatten(
+                dependentGroupsPos, plan.getSchema());
+            appendFlattens(groupsPosToFlatten, plan);
+        }
     }
     auto projection = make_shared<LogicalProjection>(expressionsToProject, plan.getLastOperator());
     projection->computeFactorizedSchema();

diff --git a/test/test_files/tinysnb/projection/single_label.test b/test/test_files/tinysnb/projection/single_label.test
@@ -725,3 +725,17 @@ cool stuff found
 happy new year
 matthew perry
 nice weather
+
+-LOG RandomFunction
+-STATEMENT match (p1:person)-[:knows]->(p2:person) WITH p1.fName AS x, p2.fName AS y, gen_random_uuid() AS z WITH DISTINCT z AS a RETURN COUNT(*);
+-ENUMERATE
+---- 1
+14
+-STATEMENT match (p1:person)-[:knows]->(p2:person) WITH DISTINCT p1.fName AS x, gen_random_uuid() AS y RETURN COUNT(*);
+-ENUMERATE
+---- 1
+14
+-STATEMENT match (p1:person)-[:knows]->(p2:person) with DISTINCT gen_random_uuid() AS x RETURN COUNT(*);
+-ENUMERATE
+---- 1
+14