-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Extract AggregationFuzzerBase from AggregationFuzzer #7916
Conversation
✅ Deploy Preview for meta-velox canceled.
|
This pull request was exported from Phabricator. Differential Revision: D51692940 |
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. Differential Revision: D51692940
0b132e2
to
9d5f34c
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
9d5f34c
to
c126fee
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
c126fee
to
4f486b6
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
4f486b6
to
5996c4c
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This is the first piece of facebookincubator#7754. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
5996c4c
to
044b938
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This is the first piece of facebookincubator#7754. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
044b938
to
a524c69
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This is the first piece of facebookincubator#7754. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
|
||
namespace facebook::velox::exec::test { | ||
|
||
class MinMaxInputGenerator : public InputGenerator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's put each of these classes into its own file or put all input generators into one file and options into its own file. BTW, input generators are specific to functions / dialect. Hence, these should not be placed into a "generic" location.
|
||
constexpr const std::string_view kPlanNodeFileName = "plan_nodes"; | ||
|
||
class InputGenerator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this into its own file. Same for ResultVerifier.
a524c69
to
0b22e60
Compare
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This is the first piece of facebookincubator#7754. Differential Revision: D51692940
This pull request was exported from Phabricator. Differential Revision: D51692940 |
…or#7916) Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This is the first piece of facebookincubator#7754. Differential Revision: D51692940
0b22e60
to
9ff2939
Compare
This pull request was exported from Phabricator. Differential Revision: D51692940 |
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Differential Revision: D51692940 fbshipit-source-id: 03c558cf62d5a755705e4cb021a00785ae57e9e3
b37a7dd
to
d813f25
Compare
Hi @mbasmanova, thank you for the review! I've addressed your comments. I've tested the build on Mac using cmake, so the red signals should be gone this time. Could you take a look again? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer and the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer. This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: 384045c5b08c2e41cffa298a86cb5b434c6d05e5
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer and the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer. This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: ad246bec3481d88527ad5df7313c9db69a229970
This pull request was exported from Phabricator. Differential Revision: D51692940 |
d813f25
to
544f4ff
Compare
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Reviewed By: mbasmanova Differential Revision: D51692940 fbshipit-source-id: 20fe59bbeb6c23f006380fcb98986a0d042cb322
This pull request was exported from Phabricator. Differential Revision: D51692940 |
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Reviewed By: mbasmanova Differential Revision: D51692940 fbshipit-source-id: 78a56f59e0c76d53b6e5ced333fd6a91f1421d66
544f4ff
to
a02bb1b
Compare
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: 029cca18ae8bb3aac2797e483b66f20e71d065c1
This pull request was exported from Phabricator. Differential Revision: D51692940 |
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Reviewed By: mbasmanova Differential Revision: D51692940 fbshipit-source-id: cf8aec101f9d0b8d47a0db1e66e5f9b651f10414
a02bb1b
to
87cb8c1
Compare
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Reviewed By: mbasmanova Differential Revision: D51692940 fbshipit-source-id: 18d6e191d46f1f3eb69e8be8c24049cbc4a25d31
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: 135b93d52c2ca43cf3e9bc8abb971a46d367d3f1
This pull request was exported from Phabricator. Differential Revision: D51692940 |
87cb8c1
to
e2a25a1
Compare
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: 8ca5f6772c43d5659f3410b49f15a0afa0d60884
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: 5e343ec133504a02e0415782f89c1b452c24d669
…or#7916) Summary: Pull Request resolved: facebookincubator#7916 Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer that reuses common logic in AggregaitonFuzzerBase. This diff also moves fuzzer-related files to velox/exec/fuzzer, including – AggregationFuzzer.h/cpp – AggregationFuzzerRunner.h – ReferenceQueryRunner.h – DuckQueryRunner.h/cpp – PrestoQueryRunner.h/cpp – InputGenerator.h – ResultVerifier.h – TransformResultVerifier.h It also moves the Presto functions' custom input generators and result verifiers to velox/functions/prestosql/fuzzer, including – MinMaxInputGenerator.h – ApproxDistinctInputGenerator.h – ApproxDistinctResultVerifier.h – ApproxPercentileInputGenerator.h – ApproxPercentileResultVerifier.h This is the first piece of facebookincubator#7754. Differential Revision: https://internalfb.com/D51692940 fbshipit-source-id: c1858f9f2a34eb292e59f80dcac4df9217c7c473
This pull request has been merged in 3206691. |
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Summary: Extract AggregationFuzzerBase from AggregationFuzzer. This is needed for building WindowFuzzer
that reuses common logic in AggregaitonFuzzerBase.
This diff also moves fuzzer-related files to
velox/exec/fuzzer, including
– AggregationFuzzer.h/cpp
– AggregationFuzzerRunner.h
– ReferenceQueryRunner.h
– DuckQueryRunner.h/cpp
– PrestoQueryRunner.h/cpp
– InputGenerator.h
– ResultVerifier.h
– TransformResultVerifier.h
It also moves the Presto functions' custom input generators and result verifiers to
velox/functions/prestosql/fuzzer, including
– MinMaxInputGenerator.h
– ApproxDistinctInputGenerator.h
– ApproxDistinctResultVerifier.h
– ApproxPercentileInputGenerator.h
– ApproxPercentileResultVerifier.h
This is the first piece of #7754.
Differential Revision: D51692940