Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic WindowFuzzerTest #7917

Closed
wants to merge 2 commits into from

Conversation

kagamiori
Copy link
Contributor

@kagamiori kagamiori commented Dec 7, 2023

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

At every iteration, this fuzzer chooses a random signature among all registered aggregation and window function signatures (excluding functions in the skipFunctions list), and execute a plan of window operation with randomly generated set of partition keys, an optional set of sorting keys (with 50% chance), and the default frame (RANGE between UNBOUNDED PRECEDING and CURRENT ROW). For functions in the orderDependentFunctions list, the set of sorting keys include the row_number column to ensure the input is fully sorted. The input columns are either generated randomly, or generated via the custom input generator if provided. A GFLAG enable_window_reference_verification (with the default value false) controls whether the result is verified against DuckDB. For functions in the customVerificationFunctions list, results are not verified even if enable_window_reference_verification is set to true.

The following GFLAGs are supported in the basic version:
enable_window_reference_verification
seed
only
steps
duration_sec
batch_size
num_batches
max_num_varargs
null_ratio
repro_persist_path
persist_and_run_once
log_signature_stats

Function stats are printed before iteration starts, e.g.,

31219 20:14:25.472924 2372852 AggregationFuzzerBase.cpp:541] Total functions: 87 (509 signatures)
I20231219 20:14:25.472947 2372852 AggregationFuzzerBase.cpp:545] Functions with at least one supported signature: 87 (100.00%)
I20231219 20:14:25.472965 2372852 AggregationFuzzerBase.cpp:551] Functions with no supported signature: 0 (0.00%)
I20231219 20:14:25.472975 2372852 AggregationFuzzerBase.cpp:554] Supported function signatures: 497 (97.64%)
I20231219 20:14:25.472982 2372852 AggregationFuzzerBase.cpp:560] Unsupported function signatures: 12 (2.36%)
I202

Test stats are printed after all iterations complete, e.g.,

I20231219 20:14:54.414773 2372852 WindowFuzzer.cpp:224] Total functions tested: 3
I20231219 20:14:54.414824 2372852 WindowFuzzer.cpp:225] Total functions requiring sorted inputs: 0 (0.00%)
I20231219 20:14:54.414860 2372852 WindowFuzzer.cpp:227] Total functions verified against reference DB: 1 (25.00%)
I20231219 20:14:54.414870 2372852 WindowFuzzer.cpp:229] Total functions not verified (verification skipped / not supported by reference DB / reference DB failed): 2 (50.00%) / 0 (0.00%) / 0 (0.00%)
I20231219 20:14:54.414880 2372852 WindowFuzzer.cpp:234] Total failed functions: 1 (25.00%)
I20231219 20:15:59.899459 2372852 AggregationFuzzerBase.cpp:344] Signature #49 failed 1 out of 1 times: approx_most_frequent( BIGINT SMALLINT BIGINT ) -> MAP<SMALLINT,BIGINT>

The window fuzzer test currently have the following limitations:

  • Not supporting reproduction of test failures from saved files.
  • Not using randomly-generated frames.
  • Not allowing custom result verifiers.
  • Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of #7754.

Differential Revision: D51880255

Copy link

netlify bot commented Dec 7, 2023

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 7875449
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/65dcd8c126b265000959e3a2

@facebook-github-bot 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 Dec 7, 2023
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 9, 2023
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

@kagamiori kagamiori changed the title Add WindowFuzzerTest Add Basic WindowFuzzerTest Dec 9, 2023
@kagamiori kagamiori changed the title Add Basic WindowFuzzerTest Add basic WindowFuzzerTest Dec 9, 2023
kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 12, 2023
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 12, 2023
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 12, 2023
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 12, 2023
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 14, 2023
Summary:
Pull Request resolved: facebookincubator#7917

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255

fbshipit-source-id: 0c9f88d9877b5d24051f66073b9996de30149ff8
kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 14, 2023
Summary:
Pull Request resolved: facebookincubator#7917

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: https://internalfb.com/D51880255

fbshipit-source-id: 4e03cf0020151f9bfe96d1d27182ad3b27f70b24
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Dec 14, 2023
Summary:
Pull Request resolved: facebookincubator#7917

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.
* Verify results against DuckDB instead of Presto.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255

fbshipit-source-id: 0f44d3cfd973c68730b8dbcef231e395b6124d3c
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 15, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 15, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

@kagamiori
Copy link
Contributor Author

Hi @aditi-pandit and @kgpai, thank you for your review! I've addressed your comments. Please take a look again. Thanks!

Copy link
Contributor

@kgpai kgpai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - can you add documentation in fuzzer.rst

++numReferenceQueryFailed;
} else if (errorCode == ReferenceQueryErrorCode::kReferenceQueryUnsupported) {
++numReferenceQueryNotSupported;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : in case we ever change it to add some other kind of failure we can do ++numReferenceQueryFailedOther.


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

class WindowFuzzerRunner {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can you add documentation for WindowFuzzer's in fuzzer.rst

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated fuzzer.rst. Please take a look and let me know any further comments. Thanks!

kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 21, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@kgpai kgpai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 23, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Reviewed By: kgpai

Differential Revision: D51880255
kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 26, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Reviewed By: kgpai

Differential Revision: D51880255
@kagamiori kagamiori force-pushed the export-D51880255 branch 2 times, most recently from a557d74 to eb65427 Compare February 26, 2024 17:58
kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 26, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Reviewed By: kgpai

Differential Revision: D51880255
kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 26, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Reviewed By: kgpai

Differential Revision: D51880255
kagamiori added a commit to kagamiori/velox that referenced this pull request Feb 26, 2024
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Reviewed By: kgpai

Differential Revision: D51880255
Summary: Pull Request resolved: facebookincubator#8681

Reviewed By: kgpai

Differential Revision: D53459530
Summary:

This diff adds a basic version of window fuzzer test that tests the Window operator with window functions and aggregation functions.

The window fuzzer test currently have the following limitations:
* Not supporting reproduction of test failures from saved files.
* Not using randomly-generated frames.
* Not allowing custom result verifiers.

The limitations will be addressed next in the subsequent PRs.

This is the second piece of facebookincubator#7754.

Reviewed By: kgpai

Differential Revision: D51880255
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 4b439db.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants