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

Improve SparkSql fuzzer proto files generation. #10810

Closed
wants to merge 1 commit into from

Conversation

zuyu
Copy link
Contributor

@zuyu zuyu commented Aug 22, 2024

A follow-up improvement for #10357 to avoid the following issue by reading proto files directly from Spark source code, instead of copying files.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	velox/functions/sparksql/fuzzer/proto/

@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 Aug 22, 2024
Copy link

netlify bot commented Aug 22, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 09cc562
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66c6f58c4304950008a321f8

@yingsu00
Copy link
Contributor

@rui-mo @FelixYBW Can you please help review this PR? Thanks!

Copy link
Collaborator

@rui-mo rui-mo left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. The extra copy could indeed be removed.

@rui-mo
Copy link
Collaborator

rui-mo commented Aug 27, 2024

cc: @kgpai @assignUser

Copy link
Collaborator

@assignUser assignUser left a comment

Choose a reason for hiding this comment

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

LGTM

@assignUser assignUser added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Aug 27, 2024
@facebook-github-bot
Copy link
Contributor

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kgpai merged this pull request in 7d5f49c.

Copy link

Conbench analyzed the 1 benchmark run on commit 7d5f49c9.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@zuyu zuyu deleted the fuzzer-cmake-fix branch August 29, 2024 00:26
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Sep 2, 2024
)

Summary:
A follow-up improvement for facebookincubator#10357 to avoid the following issue by reading proto files directly from Spark source code, instead of copying files.

```sh
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	velox/functions/sparksql/fuzzer/proto/
```

Pull Request resolved: facebookincubator#10810

Reviewed By: bikramSingh91

Differential Revision: D61867378

Pulled By: kgpai

fbshipit-source-id: e8df2cf8c590f10415a0e84bce1b640fcce159da
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Sep 3, 2024
)

Summary:
A follow-up improvement for facebookincubator#10357 to avoid the following issue by reading proto files directly from Spark source code, instead of copying files.

```sh
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	velox/functions/sparksql/fuzzer/proto/
```

Pull Request resolved: facebookincubator#10810

Reviewed By: bikramSingh91

Differential Revision: D61867378

Pulled By: kgpai

fbshipit-source-id: e8df2cf8c590f10415a0e84bce1b640fcce159da
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. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants