Generate random sort/null ordering in WindowFuzzer (facebookincubator…

…#9258) Summary: Pull Request resolved: facebookincubator#9258 Reviewed By: mbasmanova Differential Revision: D56662277 Pulled By: kagamiori fbshipit-source-id: d08990ca82a39e6ed4cadd939de003a627336e03
oerling · May 1, 2024 · 714b747 · 714b747
1 parent 8567d4d
commit 714b747
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 29 deletions.
diff --git a/velox/exec/fuzzer/WindowFuzzer.cpp b/velox/exec/fuzzer/WindowFuzzer.cpp
@@ -166,8 +166,7 @@ std::string WindowFuzzer::generateOrderByClause(
       frame << ", ";
     }
     frame << sortingKeysAndOrders[i].key_ << " "
-          << sortingKeysAndOrders[i].order_ << " "
-          << sortingKeysAndOrders[i].nullsOrder_;
+          << sortingKeysAndOrders[i].sortOrder_.toString();
   }
   return frame.str();
 }
@@ -193,11 +192,10 @@ WindowFuzzer::generateSortingKeysAndOrders(
     std::vector<TypePtr>& types) {
   auto keys = generateSortingKeys(prefix, names, types);
   std::vector<SortingKeyAndOrder> results;
-  // TODO: allow randomly generating orders.
   for (auto i = 0; i < keys.size(); ++i) {
-    std::string order = "asc";
-    std::string nullsOrder = "nulls last";
-    results.push_back(SortingKeyAndOrder(keys[i], order, nullsOrder));
+    auto asc = vectorFuzzer_.coinToss(0.5);
+    auto nullsFirst = vectorFuzzer_.coinToss(0.5);
+    results.emplace_back(keys[i], core::SortOrder(asc, nullsFirst));
   }
   return results;
 }
@@ -250,8 +248,7 @@ void WindowFuzzer::go() {
     // If the function is order-dependent or uses "rows" frame, sort all input
     // rows by row_number additionally.
     if (requireSortedInput || isRowsFrame) {
-      sortingKeysAndOrders.push_back(
-          SortingKeyAndOrder("row_number", "asc", "nulls last"));
+      sortingKeysAndOrders.emplace_back("row_number", core::kAscNullsLast);
       ++stats_.numSortedInputs;
     }
 
@@ -306,15 +303,11 @@ void WindowFuzzer::testAlternativePlans(
 
   std::vector<std::string> allKeys;
   for (const auto& key : partitionKeys) {
-    allKeys.push_back(key + " NULLS FIRST");
+    allKeys.emplace_back(key + " NULLS FIRST");
   }
   for (const auto& keyAndOrder : sortingKeysAndOrders) {
-    allKeys.push_back(folly::to<std::string>(
-        keyAndOrder.key_,
-        " ",
-        keyAndOrder.order_,
-        " ",
-        keyAndOrder.nullsOrder_));
+    allKeys.emplace_back(fmt::format(
+        "{} {}", keyAndOrder.key_, keyAndOrder.sortOrder_.toString()));
   }
 
   // Streaming window from values.

diff --git a/velox/exec/fuzzer/WindowFuzzer.h b/velox/exec/fuzzer/WindowFuzzer.h
@@ -77,20 +77,11 @@ class WindowFuzzer : public AggregationFuzzerBase {
 
  private:
   struct SortingKeyAndOrder {
-    std::string key_;
-    std::string order_;
-    std::string nullsOrder_;
-
-    SortingKeyAndOrder() = delete;
-
-    SortingKeyAndOrder(
-        const std::string& key,
-        const std::string& order,
-        const std::string& nullsOrder) {
-      key_ = key;
-      order_ = order;
-      nullsOrder_ = nullsOrder;
-    }
+    const std::string key_;
+    const core::SortOrder sortOrder_;
+
+    SortingKeyAndOrder(std::string key, core::SortOrder sortOrder)
+        : key_(std::move(key)), sortOrder_(std::move(sortOrder)) {}
   };
 
   void addWindowFunctionSignatures(const WindowFunctionMap& signatureMap);