Update code

facebookincubator · Jan 28, 2025 · fee8079 · fee8079
1 parent ee07a9d
commit fee8079
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 40 deletions.
diff --git a/velox/exec/fuzzer/CMakeLists.txt b/velox/exec/fuzzer/CMakeLists.txt
@@ -81,12 +81,6 @@ target_link_libraries(
 
 add_library(velox_row_number_fuzzer_lib RowNumberFuzzer.cpp)
 
-# RowNumber Fuzzer.
-add_executable(velox_row_number_fuzzer RowNumberFuzzerRunner.cpp)
-
-target_link_libraries(
-  velox_row_number_fuzzer velox_row_number_fuzzer_lib)
-
 target_link_libraries(
   velox_row_number_fuzzer_lib
   velox_fuzzer_util
@@ -95,6 +89,12 @@ target_link_libraries(
   velox_exec_test_lib
   velox_expression_test_utility)
 
+# RowNumber Fuzzer.
+add_executable(velox_row_number_fuzzer RowNumberFuzzerRunner.cpp)
+
+target_link_libraries(
+        velox_row_number_fuzzer velox_row_number_fuzzer_lib)
+
 add_library(velox_topn_row_number_fuzzer TopNRowNumberFuzzer.cpp)
 
 target_link_libraries(

diff --git a/velox/exec/fuzzer/DuckQueryRunner.cpp b/velox/exec/fuzzer/DuckQueryRunner.cpp
@@ -166,9 +166,9 @@ std::optional<std::string> DuckQueryRunner::toSql(
     return toSql(rowNumberNode);
   }
 
-  if (const auto rowNumberNode =
+  if (const auto topNRowNumberNode =
           std::dynamic_pointer_cast<const core::TopNRowNumberNode>(plan)) {
-    return toSql(rowNumberNode);
+    return toSql(topNRowNumberNode);
   }
 
   if (const auto joinNode =
@@ -416,7 +416,12 @@ std::optional<std::string> DuckQueryRunner::toSql(
     }
   }
 
-  sql << ") as row_number FROM tmp) ";
+  // TopNRowNumberNode should have a single source.
+  std::optional<std::string> source = toSql(rowNumberNode->sources()[0]);
+  if (!source) {
+    return std::nullopt;
+  }
+  sql << ") as row_number FROM " << *source << ") ";
 
   sql << " where row_number <= " << rowNumberNode->limit();
 

diff --git a/velox/exec/fuzzer/TopNRowNumberFuzzer.cpp b/velox/exec/fuzzer/TopNRowNumberFuzzer.cpp
@@ -238,16 +238,16 @@ TopNRowNumberFuzzer::generateKeys(const std::string& prefix) {
       INTEGER(),
       BIGINT(),
       VARCHAR(),
-      VARBINARY(),
-      TIMESTAMP(),
+      //VARBINARY(),
+      //TIMESTAMP(),
   };
 
   auto numKeys = boost::random::uniform_int_distribution<uint32_t>(1, 5)(rng_);
   std::vector<std::string> keys;
   std::vector<TypePtr> types;
   for (auto i = 0; i < numKeys; ++i) {
     keys.push_back(fmt::format("{}{}", prefix, i));
-    types.push_back(vectorFuzzer_.randOrderableType(kNonFloatingPointTypes, 2));
+    types.push_back(vectorFuzzer_.randOrderableType(kNonFloatingPointTypes, 1));
   }
 
   return std::make_pair(keys, types);
@@ -358,14 +358,7 @@ TopNRowNumberFuzzer::computeReferenceResults(
     return std::nullopt;
   }
 
-  if (auto sql = referenceQueryRunner_->toSql(plan)) {
-    LOG(INFO) << "Duckdb query generated << " << sql.value() << "; ";
-    return referenceQueryRunner_->execute(
-        sql.value(), input, plan->outputType());
-  }
-
-  LOG(INFO) << "Query not supported by the reference DB";
-  return std::nullopt;
+  return referenceQueryRunner_->execute(plan).first;
 }
 
 RowVectorPtr TopNRowNumberFuzzer::execute(
@@ -518,25 +511,25 @@ void TopNRowNumberFuzzer::verify() {
       makeDefaultPlan(partitionKeys, allSortKeys, allKeys, input);
 
   const auto expected = execute(defaultPlan, /*injectSpill=*/false);
-  if (VLOG_IS_ON(1)) {
+  /*if (VLOG_IS_ON(1)) {
     // Flatten inputs.
     const auto flatInput = flatten({expected});
     VLOG(1) << "Expected: " << expected->toString();
     for (const auto& v : flatInput) {
       VLOG(1) << std::endl << v->toString(0, v->size());
     }
-  }
-  //  if (expected != nullptr) {
-  //    if (const auto referenceResult =
-  //            computeReferenceResults(defaultPlan.plan, input)) {
-  //      VELOX_CHECK(
-  //          assertEqualResults(
-  //              referenceResult.value(),
-  //              defaultPlan.plan->outputType(),
-  //              {expected}),
-  //          "Velox and Reference results don't match");
-  //    }
-  //  }
+  }*/
+    if (expected != nullptr) {
+      if (const auto referenceResult =
+              computeReferenceResults(defaultPlan.plan, input)) {
+        VELOX_CHECK(
+            assertEqualResults(
+                referenceResult.value(),
+                defaultPlan.plan->outputType(),
+                {expected}),
+            "Velox and Reference results don't match");
+      }
+    }
 
   //  std::vector<PlanWithSplits> altPlans;
   //  altPlans.push_back(std::move(defaultPlan));

diff --git a/velox/exec/tests/CMakeLists.txt b/velox/exec/tests/CMakeLists.txt
@@ -214,13 +214,6 @@ target_link_libraries(
   velox_tpch_connector
   velox_memory)
 
-# RowNumber Fuzzer.
-add_executable(velox_row_number_fuzzer_test RowNumberFuzzerTest.cpp)
-
-target_link_libraries(
-  velox_row_number_fuzzer_test velox_row_number_fuzzer GTest::gtest
-  GTest::gtest_main)
-
 # TopNRowNumber Fuzzer.
 add_executable(velox_topn_row_number_fuzzer_test TopNRowNumberFuzzerTest.cpp)