velox icon indicating copy to clipboard operation
velox copied to clipboard

Published 20 hours ago • verified publisher icon facebookincubator

support to read binary as string in parquet

Open kevincmchen opened this issue 1 year ago • 1 comments

Summary:

Some other Parquet-producing systems, in particular Impala, will write string-type data as binary into Parquet file. and Velox's ParquetReader does not support binaryAsString. this PR is intended to resolve this issue.

issue resolved: [#10398]

kevincmchen avatar Jul 04 '24 14:07 kevincmchen

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 38119adca0fed2b33c5f311f9b589724874762c9
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66a87eddff5f210008cd8cc5

netlify[bot] avatar Jul 04 '24 14:07 netlify[bot]

@Yuhta @majetideepak the last commit caused poor performance in the Conbench performance report, besides the issue you commented. so I rewrote the code about how to get the requested type. please review it again for me. thanks

kevincmchen avatar Jul 10 '24 09:07 kevincmchen

@Yuhta could you please help me review this pr

kevincmchen avatar Jul 16 '24 02:07 kevincmchen

@majetideepak could you please help me review this pr ？

kevincmchen avatar Jul 17 '24 00:07 kevincmchen

@kevincmchen are you able to address this comment? https://github.com/facebookincubator/velox/pull/10399#discussion_r1681824613

majetideepak avatar Jul 23 '24 14:07 majetideepak

@kevincmchen are you able to address this comment? #10399 (comment) I replied to this comment and @ you, please take a look。 @majetideepak

kevincmchen avatar Jul 23 '24 15:07 kevincmchen

This change looks good. I will add the merge label after fixing the nit: auto&

I have fixed this issue nit: auto&, please take a look. @majetideepak cc @Yuhta

kevincmchen avatar Jul 30 '24 07:07 kevincmchen

@kevincmchen thanks! This should be merged soon.

majetideepak avatar Jul 30 '24 14:07 majetideepak

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

facebook-github-bot avatar Jul 30 '24 14:07 facebook-github-bot

@kevincmchen thanks! This should be merged soon.

Thank you for helping me review code and provide a lot of good suggestions! @majetideepak @Yuhta

kevincmchen avatar Jul 30 '24 15:07 kevincmchen

@Yuhta merged this pull request in facebookincubator/velox@e47f2d46c031e5edeff60395c346b3f8d372553f.

facebook-github-bot avatar Jul 30 '24 16:07 facebook-github-bot

Conbench analyzed the 1 benchmark run on commit e47f2d46.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

conbench-facebook[bot] avatar Jul 30 '24 17:07 conbench-facebook[bot]