Avoid generating unsupported data types for JoinFuzzer with PQR #10662

kagamiori · 2024-08-05T15:51:00Z

Summary:
When running JoinFuzzerTest with PrestoQueryRunner, currently only about
16-20% iterations are verified against Presto. The rest iterations are
unverified due to unsupported data types. This diff makes JoinFuzzer to
avoid generating unsupported types when running with PrestoQueryRunner.
After this change, over 85% of iterations are verified against Presto.

Differential Revision: D60768414

netlify · 2024-08-05T15:51:18Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`d9d92aa`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/66bece2569bb0b0008b461d4

facebook-github-bot · 2024-08-05T15:51:26Z

This pull request was exported from Phabricator. Differential Revision: D60768414

facebook-github-bot · 2024-08-06T20:59:38Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…ncubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, about 80% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-06T21:35:03Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…ncubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-06T21:40:39Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…ncubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-13T17:13:34Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…ncubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-13T17:28:15Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…ncubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-13T20:57:30Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…ncubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

xiaoxmeng

@kagamiori LGTM % minors. Thanks!

xiaoxmeng · 2024-08-14T04:32:29Z

velox/vector/fuzzer/VectorFuzzer.h

@@ -389,4 +389,7 @@ RowTypePtr randRowType(
    const std::vector<TypePtr>& scalarTypes,
    int maxDepth = 5);

+// Default set of scalar types to be chosen from when generating random types.


/// for public comment

xiaoxmeng · 2024-08-14T04:37:21Z

velox/exec/fuzzer/ReferenceQueryRunner.h


 namespace facebook::velox::exec::test {

+enum RunnerType { kPrestoQueryRunner, kDuckQueryRunner };
+
 /// Query runner that uses reference database, i.e. DuckDB, Presto, Spark.
 class ReferenceQueryRunner {


class ReferenceQueryRunner { public: enum class Type { kPresto, kDuckDB, }

xiaoxmeng · 2024-08-14T04:38:58Z

velox/exec/fuzzer/PrestoQueryRunner.h

+    return RunnerType::kPrestoQueryRunner;
+  }
+
+  const std::vector<TypePtr>& supportedScalarTypes() const override {


nit: move the nontrivial implementation to cc? thanks!

xiaoxmeng · 2024-08-14T04:39:14Z

velox/exec/fuzzer/DuckQueryRunner.h

+  /// casting from Bigint to Interval.
+  ///
+  /// TODO Investigate mismatches reported when comparing Varbinary.
+  const std::vector<TypePtr>& supportedScalarTypes() const override {


xiaoxmeng · 2024-08-14T04:40:47Z

velox/exec/fuzzer/JoinFuzzer.cpp

+    // The number of iterations that test cross product.
+    size_t numCrossProduct{0};
+
+    void print(size_t totalIterations) const {


Let make totalIterations a member?
size_t numIterations;

And add a toString() method for stats
LOG(INFO) << stats_.toString()?
Thanks!

So I have tried making this change, but it turns out to be less readable then before. The original output is

W0814 12:39:44.775323 847942 JoinFuzzer.cpp:1481] ==============================> Done with iteration 0 I0814 12:39:44.775427 847942 JoinFuzzer.cpp:318] Total iterations tested: 1 I0814 12:39:44.775455 847942 JoinFuzzer.cpp:319] Total iterations verified against reference DB: 1 (100.00%) I0814 12:39:44.775501 847942 JoinFuzzer.cpp:321] Total iterations testing cross product: 0 (0.00%)

After changing the code to LOG(INFO) << stats_.toString(), the output becomes

W0814 12:36:05.199414 779680 JoinFuzzer.cpp:1485] ==============================> Done with iteration 3 I0814 12:36:05.199489 779680 JoinFuzzer.cpp:1491] Total iterations tested: 4 Total iterations verified against reference DB: 3 (75.00%) Total iterations testing cross product: 0 (0.00%)

I think the original output is better, so I keep the code.

xiaoxmeng · 2024-08-14T04:41:21Z

velox/exec/fuzzer/JoinFuzzer.cpp

@@ -350,7 +372,8 @@ std::vector<TypePtr> JoinFuzzer::generateJoinKeyTypes(int32_t numKeys) {
  types.reserve(numKeys);
  for (auto i = 0; i < numKeys; ++i) {
    // Pick random scalar type.
-    types.push_back(vectorFuzzer_.randType(0 /*maxDepth*/));
+    types.push_back(vectorFuzzer_.randType(
+        referenceQueryRunner_->supportedScalarTypes(), 0 /*maxDepth*/));


/*maxDepth=*/0

xiaoxmeng · 2024-08-14T04:41:30Z

velox/exec/fuzzer/JoinFuzzer.cpp

@@ -373,7 +396,8 @@ std::vector<RowVectorPtr> JoinFuzzer::generateProbeInput(
  const auto numPayload = randInt(0, 3);
  for (auto i = 0; i < numPayload; ++i) {
    names.push_back(fmt::format("tp{}", i + keyNames.size()));
-    types.push_back(vectorFuzzer_.randType(2 /*maxDepth*/));
+    types.push_back(vectorFuzzer_.randType(
+        referenceQueryRunner_->supportedScalarTypes(), 2 /*maxDepth*/));


xiaoxmeng · 2024-08-14T04:42:18Z

velox/exec/fuzzer/JoinFuzzer.cpp

@@ -406,7 +430,8 @@ std::vector<RowVectorPtr> JoinFuzzer::generateBuildInput(
  const auto numPayload = randInt(0, 3);
  for (auto i = 0; i < numPayload; ++i) {
    names.push_back(fmt::format("bp{}", i + buildKeys.size()));
-    types.push_back(vectorFuzzer_.randType(2 /*maxDepth*/));
+    types.push_back(vectorFuzzer_.randType(
+        referenceQueryRunner_->supportedScalarTypes(), 2 /*maxDepth*/));


velox/exec/fuzzer/JoinFuzzer.cpp

xiaoxmeng · 2024-08-14T04:43:14Z

velox/exec/fuzzer/JoinFuzzer.cpp

@@ -933,6 +955,9 @@ RowVectorPtr JoinFuzzer::testCrossProduct(
          assertEqualResults(
              referenceResult.value(), plan.plan->outputType(), {expected}),
          "Velox and DuckDB results don't match");
+
+      LOG(INFO) << "Result matches with referenc DB.";


facebook-github-bot · 2024-08-14T19:31:27Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…bookincubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-14T19:42:25Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…bookincubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-14T19:47:56Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…bookincubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

facebook-github-bot · 2024-08-14T19:53:04Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…bookincubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Differential Revision: D60768414

xiaoxmeng

@kagamiori thanks for the update % nits.

xiaoxmeng · 2024-08-14T20:15:34Z

velox/exec/fuzzer/ReferenceQueryRunner.h


 namespace facebook::velox::exec::test {

 /// Query runner that uses reference database, i.e. DuckDB, Presto, Spark.
 class ReferenceQueryRunner {
 public:
+  enum RunnerType { kPrestoQueryRunner, kDuckQueryRunner };


Can we just define it as enum class?

xiaoxmeng · 2024-08-14T20:18:13Z

velox/exec/fuzzer/JoinFuzzer.cpp

+    // The number of iterations that test cross product.
+    size_t numCrossProduct{0};
+
+    void print() const {


The common practice is to provide a toString method and let the control path to log out if need.

xiaoxmeng · 2024-08-14T20:18:35Z

velox/exec/fuzzer/JoinFuzzer.cpp

+
+  struct Stats {
+    // The total number of iterations tested.
+    size_t totalIterations{0};


s/totalIterations/numIterations/

The same naming as the other two? Thanks!

facebook-github-bot · 2024-08-14T21:44:15Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…bookincubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Reviewed By: xiaoxmeng Differential Revision: D60768414

facebook-github-bot · 2024-08-15T18:30:34Z

This pull request was exported from Phabricator. Differential Revision: D60768414

…bookincubator#10662) Summary: Pull Request resolved: facebookincubator#10662 When running JoinFuzzerTest with PrestoQueryRunner, currently only about 16-20% iterations are verified against Presto. The rest iterations are unverified due to unsupported data types. This diff makes JoinFuzzer to avoid generating unsupported types when running with PrestoQueryRunner. After this change, over 85% of iterations are verified against Presto. Reviewed By: xiaoxmeng Differential Revision: D60768414

facebook-github-bot · 2024-08-16T03:57:21Z

This pull request was exported from Phabricator. Differential Revision: D60768414

facebook-github-bot · 2024-08-16T15:35:47Z

This pull request has been merged in d23dde5.

conbench-facebook · 2024-08-16T16:03:18Z

Conbench analyzed the 1 benchmark run on commit d23dde5a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 5, 2024

facebook-github-bot added the fb-exported label Aug 5, 2024

kagamiori force-pushed the export-D60768414 branch from ab2f06d to 49bc4b3 Compare August 6, 2024 20:59

kagamiori force-pushed the export-D60768414 branch from 49bc4b3 to b5efb92 Compare August 6, 2024 21:35

kagamiori force-pushed the export-D60768414 branch 2 times, most recently from 8f99711 to 00c69f7 Compare August 13, 2024 17:13

kagamiori force-pushed the export-D60768414 branch from 00c69f7 to c09905f Compare August 13, 2024 17:28

kagamiori force-pushed the export-D60768414 branch from c09905f to e4fac78 Compare August 13, 2024 20:57

kagamiori changed the title ~~Avoid generating unsupported types for JoinFuzzer with PQR~~ Avoid generating unsupported data types for JoinFuzzer with PQR Aug 13, 2024

xiaoxmeng reviewed Aug 14, 2024

View reviewed changes

kagamiori force-pushed the export-D60768414 branch from e4fac78 to 2afd736 Compare August 14, 2024 19:31

kagamiori force-pushed the export-D60768414 branch from 2afd736 to 44321d3 Compare August 14, 2024 19:42

kagamiori force-pushed the export-D60768414 branch from 44321d3 to 307a742 Compare August 14, 2024 19:48

kagamiori force-pushed the export-D60768414 branch from 307a742 to 0f8365f Compare August 14, 2024 19:53

xiaoxmeng approved these changes Aug 14, 2024

View reviewed changes

kagamiori force-pushed the export-D60768414 branch from 0f8365f to cf6b3f0 Compare August 14, 2024 21:44

kagamiori force-pushed the export-D60768414 branch from cf6b3f0 to 26b156e Compare August 15, 2024 18:30

kagamiori force-pushed the export-D60768414 branch from 26b156e to d9d92aa Compare August 16, 2024 03:57

facebook-github-bot closed this in d23dde5 Aug 16, 2024

facebook-github-bot added the Merged label Aug 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid generating unsupported data types for JoinFuzzer with PQR #10662

Avoid generating unsupported data types for JoinFuzzer with PQR #10662

kagamiori commented Aug 5, 2024 •

edited

Loading

netlify bot commented Aug 5, 2024 •

edited

Loading

facebook-github-bot commented Aug 5, 2024

facebook-github-bot commented Aug 6, 2024

facebook-github-bot commented Aug 6, 2024

facebook-github-bot commented Aug 6, 2024

facebook-github-bot commented Aug 13, 2024

facebook-github-bot commented Aug 13, 2024

facebook-github-bot commented Aug 13, 2024

xiaoxmeng left a comment

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

kagamiori Aug 14, 2024 •

edited

Loading

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

xiaoxmeng left a comment

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

xiaoxmeng Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 15, 2024

facebook-github-bot commented Aug 16, 2024

facebook-github-bot commented Aug 16, 2024

conbench-facebook bot commented Aug 16, 2024

Avoid generating unsupported data types for JoinFuzzer with PQR #10662

Avoid generating unsupported data types for JoinFuzzer with PQR #10662

Conversation

kagamiori commented Aug 5, 2024 • edited Loading

netlify bot commented Aug 5, 2024 • edited Loading

✅ Deploy Preview for meta-velox canceled.

facebook-github-bot commented Aug 5, 2024

facebook-github-bot commented Aug 6, 2024

facebook-github-bot commented Aug 6, 2024

facebook-github-bot commented Aug 6, 2024

facebook-github-bot commented Aug 13, 2024

facebook-github-bot commented Aug 13, 2024

facebook-github-bot commented Aug 13, 2024

xiaoxmeng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kagamiori Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 14, 2024

xiaoxmeng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Aug 14, 2024

facebook-github-bot commented Aug 15, 2024

facebook-github-bot commented Aug 16, 2024

facebook-github-bot commented Aug 16, 2024

conbench-facebook bot commented Aug 16, 2024

kagamiori commented Aug 5, 2024 •

edited

Loading

netlify bot commented Aug 5, 2024 •

edited

Loading

kagamiori Aug 14, 2024 •

edited

Loading