-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(parquet): Use velox parquet reader in StatisticsTest and Metadatatest #11472
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for meta-velox canceled.
|
4af254d
to
c97061c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkhaliqi can you move the file from arrow tests to here so that we can only see the Velox specific changes?
return readerBase_->thriftFileMetaDatas(); | ||
} | ||
|
||
void WriteBufferToFile(const std::shared_ptr<arrow::Buffer>& buffer, const std::string& file_path) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this here as this only used for testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will change this methods location since it is only for testing before changing from draft PR, for now will use this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted whole function
@@ -74,6 +75,10 @@ class ReaderBase { | |||
return *fileMetaData_; | |||
} | |||
|
|||
std::shared_ptr<thrift::FileMetaData> thriftFileMetaDatas() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use thriftFileMetaData()
above. We don't need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use above function, thank you!
d62718d
to
495598e
Compare
} | ||
|
||
void WriteBufferToFile(const std::shared_ptr<arrow::Buffer>& buffer, const std::string& file_path) { | ||
auto source = std::make_shared<::arrow::io::BufferReader>(buffer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simply write buffer()
(returns a std::string_view) to a file using LocalWriteFile
. See S3FileSystemTest.cpp for example.
ASSERT_NE(nullptr, file_reader->metadata()->key_value_metadata()); | ||
auto read_kv_meta = file_reader->metadata()->key_value_metadata(); | ||
// Write the buffer to a temp file | ||
std::string file_path = "output_file.parquet"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use TempFilePath
.
8aa9724
to
85013aa
Compare
287d618
to
d5f7423
Compare
d5f7423
to
ece201e
Compare
ece201e
to
abc8f1d
Compare
abc8f1d
to
c862477
Compare
No description provided.