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

Fix parsing array struct when repetition_type is 'REPEATED' #10753

Closed
wants to merge 1 commit into from

Conversation

yma11
Copy link
Contributor

@yma11 yma11 commented Aug 14, 2024

Velox fails to parse array struct in attached example file as it doesn't consider RowType in when repetition_type is REPEATED.

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

netlify bot commented Aug 14, 2024

Deploy Preview for meta-velox canceled.

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

Copy link
Contributor

@Yuhta Yuhta left a comment

Choose a reason for hiding this comment

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

Can you add a test? Since you already check in the data file, I guess you just miss checking in the test code

@yma11 yma11 changed the title Fix parsing array struct when repetition_type is 'REPEATED' [WIP] Fix parsing array struct when repetition_type is 'REPEATED' Aug 15, 2024
@yma11
Copy link
Contributor Author

yma11 commented Aug 15, 2024

Can you add a test? Since you already check in the data file, I guess you just miss checking in the test code

Updated. But seems data read also have some problems. Let me do try a further fix.

@yma11
Copy link
Contributor Author

yma11 commented Aug 19, 2024

@Yuhta Fixed. Please help review again. Thanks.

@yma11 yma11 changed the title [WIP] Fix parsing array struct when repetition_type is 'REPEATED' Fix parsing array struct when repetition_type is 'REPEATED' Aug 20, 2024
@yma11 yma11 force-pushed the array-struct branch 2 times, most recently from b345f25 to ca70196 Compare August 23, 2024 01:46
@Yuhta Yuhta 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 23, 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 f996a0b.

Copy link

Conbench analyzed the 1 benchmark run on commit f996a0b7.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Sep 2, 2024
…incubator#10753)

Summary:
Velox fails to parse array struct in attached example file as it doesn't consider `RowType` in when `repetition_type` is `REPEATED`.

Pull Request resolved: facebookincubator#10753

Reviewed By: Yuhta

Differential Revision: D61862384

Pulled By: kgpai

fbshipit-source-id: c15338630a0a952acc80be6c2e91857e42cb9350
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Sep 3, 2024
…incubator#10753)

Summary:
Velox fails to parse array struct in attached example file as it doesn't consider `RowType` in when `repetition_type` is `REPEATED`.

Pull Request resolved: facebookincubator#10753

Reviewed By: Yuhta

Differential Revision: D61862384

Pulled By: kgpai

fbshipit-source-id: c15338630a0a952acc80be6c2e91857e42cb9350
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.

3 participants