feat(fuzzer): Support custom input generator in VectorFuzzer #11466

kagamiori · 2024-11-07T01:57:37Z

Summary:
Custom types often require custom logic to generate valid values, such as JSON. To support such
custom data generation for expression fuzzer, this diff makes two changes:

Require a custom type to provide a custom input generator that is automatically used when
VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case
VectorFuzzer generates random data in the old way.
Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be
needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions
that require some arguments to be positive numbers).

Differential Revision: D65576377

netlify · 2024-11-07T01:57:54Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`67691da`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/6789c1fe8f678d000867bb24

facebook-github-bot · 2024-11-07T01:57:58Z

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

facebook-github-bot · 2024-11-07T18:21:08Z

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

…bator#11466) Summary: Pull Request resolved: facebookincubator#11466 DO NOT REVIEW NOW. Differential Revision: D65576377

facebook-github-bot · 2024-11-07T18:30:34Z

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

…bator#11466) Summary: Pull Request resolved: facebookincubator#11466 DO NOT REVIEW NOW. Differential Revision: D65576377

facebook-github-bot · 2024-11-07T18:43:48Z

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

assignUser · 2024-11-07T22:45:18Z

Please set WIP PRs into draft mode (does not notify codeowners) until you want a review to reduce notification volume for codeowners :)

kagamiori · 2024-11-08T03:34:13Z

Please set WIP PRs into draft mode (does not notify codeowners) until you want a review to reduce notification volume for codeowners :)

Hi @assignUser, I tried but unfortunately couldn't find the convert-to-draft button from my side. I guess it's because this PR was exported from our internal system.

By the way, the failed CI jobs report an error during Configure Build as follows:

CMake Error at velox/experimental/fuzzer_input_generator/CMakeLists.txt:26 (target_compile_options):
target_compile_options can not be used on an ALIAS target.

But the reported CMakeLists file doesn't use an ALIAS target with target_compile_options. Do you know how I could address this error?

velox_add_library(
  velox_fuzzer_constrained_input_generators ConstrainedGenerators.cpp
  ConstrainedVectorGenerator.cpp)

velox_link_libraries(
  velox_fuzzer_constrained_input_generators
  Folly::folly
  velox_expression
  velox_type
  velox_vector_fuzzer_util)

if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
  target_compile_options(velox_fuzzer_constrained_input_generators
                         PRIVATE -Wno-deprecated-declarations)
endif()

assignUser · 2024-11-08T14:58:58Z

But the reported CMakeLists file doesn't use an ALIAS target with target_compile_options. Do you know how I could address this error?

The mono library creates alias targets for all targets, so you have to check with if(VELOX_MONO_LIBRARY) and use velox as the target. Or if it's only one TU with the warnings you can set the flag on that source file with https://cmake.org/cmake/help/latest/command/set_source_files_properties.html this would then not set the flag on all files in velox.

…bator#11466) Summary: Pull Request resolved: facebookincubator#11466 DO NOT REVIEW NOW. Differential Revision: D65576377

facebook-github-bot · 2024-11-12T19:26:10Z

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

kagamiori · 2024-11-13T18:41:42Z

But the reported CMakeLists file doesn't use an ALIAS target with target_compile_options. Do you know how I could address this error?

The mono library creates alias targets for all targets, so you have to check with if(VELOX_MONO_LIBRARY) and use velox as the target. Or if it's only one TU with the warnings you can set the flag on that source file with https://cmake.org/cmake/help/latest/command/set_source_files_properties.html this would then not set the flag on all files in velox.

Hi @assignUser, could you please share an example of the suggested approach? I tried adding the following code in the CMakeLists.txt file but it didn't work on my Mac (i.e., I still see errors of using the deprecated thing).

if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
    set_source_files_properties(Utils.h PROPERTIES COMPILE_OPTIONS "-Wno-deprecated-declarations")
    set_source_files_properties(Utils.cpp PROPERTIES COMPILE_OPTIONS "-Wno-deprecated-declarations")
endif()

I've also tried adding #pragma clang diagnostic ignored "-Wdeprecated-declarations" in Utils.h and Utils.cpp that have the deprecated usage. It worked on my Mac but fails in CI (https://github.com/facebookincubator/velox/actions/runs/11804302396/job/32884198994?pr=11466).

…bator#11466) Summary: Pull Request resolved: facebookincubator#11466 DO NOT REVIEW NOW. Differential Revision: D65576377

facebook-github-bot · 2024-11-15T02:35:12Z

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

…bator#11466) Summary: Pull Request resolved: facebookincubator#11466 DO NOT REVIEW NOW. Differential Revision: D65576377

facebook-github-bot · 2024-11-16T01:09:12Z

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

assignUser

Why should the generator that, afaiu is only supposed to be used for fuzzing be part of libvelox and link to the actual fuzzer? As type is core we would ALWAYS need to build with fuzzers?

assignUser · 2025-01-10T22:36:05Z

velox/functions/prestosql/types/JsonType.cpp

+        config.seed_,
+        JSON(),
+        config.nullRatio_,
+        fuzzer::getRandomInputGenerator(


This is the cause of the missing symbol. The link to velox_fuzzer... is removed by the monolithic wrapper function. We could link this explicitly but to be honest I don't think the type defintion is the right place for an input generator?

Hi @assignUser, every custom type needs a logic to generate valid data, so the custom types and input generators are naturally bounded. So we add a new API getInputGenerator() to CustomTypeFactories to remind every custom type author to define the data generation logic intentionally. I'll try to move ConstrainedInputGenerators.h/cpp to another place not under fuzzer directories. Thank you for helping pinpoint the problem!

I'm moving ConstrainedInputGenerators.h/cpp to velox/common/fuzzer: #12080.

Summary: Presto may perform constant folding on queries before sending the fragment to velox workers. However, when the workers receive the fragments, the fragments may contain types which had a different implementation than how velox implemented the type. This incompatibility results in incorrect results. For example, this PR fixes the type incompatibility between Java coordinator and C++ worker for `ipaddress` types. - Java coordinator, ipaddress is represented as a slice of 16 bytes which if represented as a number, would be big endian. - C++ worker, ipaddress is represented as an int128_t, in little endian form. The discrepancy between these two can be see with on native engine, the result set will be `::ffff:1.2.3.4` represented in reverse byte order ``` SELECT CAST(ip AS ipaddress) as casted_ip FROM ( VALUES ('::ffff:1.2.3.4') ) AS t (ip) ``` To address this issue, we can reverse the byte order of the ipaddress type sent from and to Java. **Note**: - This issue is not exclusive to ipaddrss, and other custom types in velox which have different underlying type/implementation than Java may suffer from this issue as well. - We can likely enhance the fuzzer to help catch cases like this at diff time once custom fuzzer inputs are landed (facebookincubator#11466) Differential Revision: D68039050

…kincubator#11466) Summary: Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

facebook-github-bot · 2025-01-14T20:01:12Z

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

facebook-github-bot · 2025-01-14T23:45:01Z

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

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

facebook-github-bot · 2025-01-15T20:36:05Z

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

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

facebook-github-bot · 2025-01-15T20:53:11Z

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

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

kevinwilfong · 2025-01-15T23:48:27Z

velox/type/Type.h

+  // Type of data represented by JSON. This config should be ignored by non-JSON
+  // input generators.
+  const TypePtr& representedType_;


Could we just let the JSON Generator call velox::randType(...) with a FuzzerGenerator created from seed_?

Alternatively, JSON really only supports a handful of types: string, integer, float, object (map), array, and boolean (any of which can be null), so JSON could just randomly construct one of those if you don't want to link against VectorFuzzer in the Presto Type code.

kevinwilfong · 2025-01-15T23:49:04Z

velox/vector/fuzzer/VectorFuzzer.cpp

@@ -336,13 +355,16 @@ VectorPtr VectorFuzzer::fuzzConstant(const TypePtr& type, vector_size_t size) {
        opts_.maxConstantContainerSize.value(), opts_.containerLength);
    opts_.complexElementsMaxSize = std::min<int32_t>(
        opts_.maxConstantContainerSize.value(), opts_.complexElementsMaxSize);
+    // TODO: incorporate fuzzer options into customGenerator.


This comment feels like it would be more appropriate where we create the InputGeneratorConfig in VectorFuzzer.cpp.

facebook-github-bot · 2025-01-16T20:57:51Z

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

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

Differential Revision: D68137136

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

facebook-github-bot · 2025-01-17T01:22:55Z

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

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

facebook-github-bot · 2025-01-17T02:35:37Z

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

kevinwilfong

Thanks! LGTM

…kincubator#11466) Summary: Pull Request resolved: facebookincubator#11466 Custom types often require custom logic to generate valid values, such as JSON. To support such custom data generation for expression fuzzer, this diff makes two changes: 1. Require a custom type to provide a custom input generator that is automatically used when VectorFuzzer generates vectors of this type. The custom type can provide a nullptr, in which case VectorFuzzer generates random data in the old way. 2. Allow users of VectorFuzzer to provide a custom input generator to the API calls. (This will be needed for custom input generation for non-custom types in expression fuzzer, such as cdf functions that require some arguments to be positive numbers). Differential Revision: D65576377

kagamiori requested review from assignUser and majetideepak as code owners November 7, 2024 01:57

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 Nov 7, 2024

facebook-github-bot added the fb-exported label Nov 7, 2024

kagamiori changed the title ~~Add input generator for json_parse in expression fuzzer~~ [WIP] Add input generator for json_parse in expression fuzzer Nov 7, 2024

kagamiori force-pushed the export-D65576377 branch from b69e0d2 to 6e3c393 Compare November 7, 2024 18:21

kagamiori force-pushed the export-D65576377 branch from 6e3c393 to e283b96 Compare November 7, 2024 18:30

kagamiori force-pushed the export-D65576377 branch from e283b96 to 595c9a0 Compare November 7, 2024 18:43

assignUser marked this pull request as draft November 8, 2024 14:59

kagamiori force-pushed the export-D65576377 branch from 595c9a0 to 4ec5014 Compare November 12, 2024 19:26

kagamiori force-pushed the export-D65576377 branch from 4ec5014 to d12b361 Compare November 15, 2024 02:35

kagamiori force-pushed the export-D65576377 branch from d12b361 to 9b2d277 Compare November 16, 2024 01:09

assignUser requested changes Jan 10, 2025

View reviewed changes

yuandagits mentioned this pull request Jan 11, 2025

fix: Address incompatibility of constant folding for ipaddress #12065

Closed

kagamiori force-pushed the export-D65576377 branch from 6b0bf0e to 6ba8691 Compare January 14, 2025 20:00

kagamiori force-pushed the export-D65576377 branch from 6ba8691 to 2e51e14 Compare January 14, 2025 23:45

kagamiori force-pushed the export-D65576377 branch from 2e51e14 to ed17783 Compare January 15, 2025 20:36

kagamiori force-pushed the export-D65576377 branch from ed17783 to 37b0528 Compare January 15, 2025 20:53

kevinwilfong reviewed Jan 15, 2025

View reviewed changes

kagamiori force-pushed the export-D65576377 branch from 37b0528 to 09f893c Compare January 16, 2025 20:57

misc: Move ConstrainedInputGenerator to velox/common/fuzzer

215d8b6

Differential Revision: D68137136

kagamiori force-pushed the export-D65576377 branch from 09f893c to c6a645b Compare January 17, 2025 01:22

kagamiori force-pushed the export-D65576377 branch from c6a645b to 67691da Compare January 17, 2025 02:35

kagamiori requested a review from kevinwilfong January 17, 2025 17:15

kevinwilfong approved these changes Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(fuzzer): Support custom input generator in VectorFuzzer #11466

feat(fuzzer): Support custom input generator in VectorFuzzer #11466

kagamiori commented Nov 7, 2024 •

edited

Loading

netlify bot commented Nov 7, 2024 •

edited

Loading

facebook-github-bot commented Nov 7, 2024

facebook-github-bot commented Nov 7, 2024

facebook-github-bot commented Nov 7, 2024

facebook-github-bot commented Nov 7, 2024

assignUser commented Nov 7, 2024

kagamiori commented Nov 8, 2024

assignUser commented Nov 8, 2024

facebook-github-bot commented Nov 12, 2024

kagamiori commented Nov 13, 2024

facebook-github-bot commented Nov 15, 2024

facebook-github-bot commented Nov 16, 2024

assignUser left a comment

assignUser Jan 10, 2025

kagamiori Jan 13, 2025

kagamiori Jan 14, 2025

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 15, 2025

facebook-github-bot commented Jan 15, 2025

kevinwilfong Jan 15, 2025

kevinwilfong Jan 15, 2025

kevinwilfong Jan 15, 2025

facebook-github-bot commented Jan 16, 2025

facebook-github-bot commented Jan 17, 2025

facebook-github-bot commented Jan 17, 2025

kevinwilfong left a comment

feat(fuzzer): Support custom input generator in VectorFuzzer #11466

Are you sure you want to change the base?

feat(fuzzer): Support custom input generator in VectorFuzzer #11466

Conversation

kagamiori commented Nov 7, 2024 • edited Loading

netlify bot commented Nov 7, 2024 • edited Loading

✅ Deploy Preview for meta-velox canceled.

facebook-github-bot commented Nov 7, 2024

facebook-github-bot commented Nov 7, 2024

facebook-github-bot commented Nov 7, 2024

facebook-github-bot commented Nov 7, 2024

assignUser commented Nov 7, 2024

kagamiori commented Nov 8, 2024

assignUser commented Nov 8, 2024

facebook-github-bot commented Nov 12, 2024

kagamiori commented Nov 13, 2024

facebook-github-bot commented Nov 15, 2024

facebook-github-bot commented Nov 16, 2024

assignUser left a comment

Choose a reason for hiding this comment

assignUser Jan 10, 2025

Choose a reason for hiding this comment

kagamiori Jan 13, 2025

Choose a reason for hiding this comment

kagamiori Jan 14, 2025

Choose a reason for hiding this comment

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 15, 2025

facebook-github-bot commented Jan 15, 2025

kevinwilfong Jan 15, 2025

Choose a reason for hiding this comment

kevinwilfong Jan 15, 2025

Choose a reason for hiding this comment

kevinwilfong Jan 15, 2025

Choose a reason for hiding this comment

facebook-github-bot commented Jan 16, 2025

facebook-github-bot commented Jan 17, 2025

facebook-github-bot commented Jan 17, 2025

kevinwilfong left a comment

Choose a reason for hiding this comment

kagamiori commented Nov 7, 2024 •

edited

Loading

netlify bot commented Nov 7, 2024 •

edited

Loading