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

refactor: Enable Parquet and Arrow by default #11832

Closed

Conversation

zuyu
Copy link
Contributor

@zuyu zuyu commented Dec 11, 2024

Suggested by @majetideepak in #11698 (comment).

Enable Parquet by default, which needs Arrow, thus Arrow is also enabled by default.

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

netlify bot commented Dec 11, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 21df178
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/675b00fe00e0db0008f37511

@zuyu zuyu marked this pull request as draft December 12, 2024 01:44
@zuyu zuyu force-pushed the parquet-arrow-enable-by-default branch from 43dc9d0 to a99a5fb Compare December 12, 2024 03:22
@zuyu zuyu marked this pull request as ready for review December 12, 2024 03:44
@assignUser
Copy link
Collaborator

Do we really want to remove the entire option to build velox without parquet/arrow? I haven't followed the prior PR if this was discussed. Is it now used in the core of velox? @majetideepak you commented on this in the other PR?

@zuyu zuyu force-pushed the parquet-arrow-enable-by-default branch from a99a5fb to b342ac0 Compare December 12, 2024 15:14
@majetideepak
Copy link
Collaborator

My comment #11698 (comment) was misunderstood.
I meant we could remove ifdef VELOX_ENABLE_PARQUET in the file velox/connectors/hive/HiveConnectorUtil.cpp. This was likely added for Meta's buck. So only a Meta developer can validate that.

@zuyu zuyu closed this Dec 12, 2024
@zuyu zuyu deleted the parquet-arrow-enable-by-default branch December 12, 2024 15:34
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants