github-actions[bot] commented on code in PR #45562: URL: https://github.com/apache/doris/pull/45562#discussion_r1889082187
########## be/src/vec/exec/format/parquet/parquet_column_chunk_file_reader.cpp: ########## @@ -0,0 +1,213 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "vec/exec/format/parquet/parquet_column_chunk_file_reader.h" + +#include <cstring> +#include <stdexcept> + +namespace doris { +namespace vectorized { Review Comment: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] ```suggestion namespace doris::vectorized { ``` be/src/vec/exec/format/parquet/parquet_column_chunk_file_reader.cpp:-1: ```diff - eturn Status::OK(); + eturn Status::OK();} ``` ########## be/src/vec/exec/format/parquet/reference_counted_reader.h: ########## @@ -0,0 +1,92 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <cstdint> +#include <memory> +#include <string> + +#include "io/fs/buffered_reader.h" +#include "io/fs/file_reader.h" + +namespace doris { +namespace vectorized { Review Comment: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] ```suggestion namespace doris::vectorized { ``` be/src/vec/exec/format/parquet/reference_counted_reader.h:90: ```diff - _count; - vectorized + _count; + vec} ``` ########## be/src/vec/exec/format/parquet/parquet_column_chunk_file_reader.h: ########## @@ -0,0 +1,114 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <iterator> +#include <memory> +#include <vector> + +#include "common/status.h" +#include "io/fs/file_reader.h" +#include "io/fs/path.h" +#include "util/slice.h" +#include "vec/exec/format/parquet/reference_counted_reader.h" + +namespace doris { +namespace vectorized { Review Comment: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] ```suggestion namespace doris::vectorized { ``` be/src/vec/exec/format/parquet/parquet_column_chunk_file_reader.h:112: ```diff - } // namespace vectorized - } // namespace doris + } // namespace doris ``` ########## be/src/vec/exec/format/parquet/reference_counted_reader.cpp: ########## @@ -0,0 +1,149 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "vec/exec/format/parquet/reference_counted_reader.h" + +#include <sstream> +#include <stdexcept> + +namespace doris { +namespace vectorized { Review Comment: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] ```suggestion namespace doris::vectorized { ``` be/src/vec/exec/format/parquet/reference_counted_reader.cpp:148: ```diff + } ``` ########## be/src/vec/exec/format/parquet/parquet_column_chunk_file_reader.h: ########## @@ -0,0 +1,114 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include <iterator> +#include <memory> +#include <vector> + +#include "common/status.h" +#include "io/fs/file_reader.h" +#include "io/fs/path.h" +#include "util/slice.h" +#include "vec/exec/format/parquet/reference_counted_reader.h" + +namespace doris { +namespace vectorized { + +/** + * A FileReader implementation for reading parquet column chunks. + * It reads column chunks in limited (small) byte chunks (8MB by default). + * Column chunks consists of multiple pages. + * This abstraction is used because the page size is unknown until the page header is read + * and page header and page data can be split between two or more byte chunks. + */ +class ParquetColumnChunkFileReader : public io::FileReader { +public: + ParquetColumnChunkFileReader(std::vector<std::shared_ptr<ChunkReader>> chunks, + ChunkReader::Statistics& statistics); + ~ParquetColumnChunkFileReader() override; + + // Prevent copying + ParquetColumnChunkFileReader(const ParquetColumnChunkFileReader&) = delete; + ParquetColumnChunkFileReader& operator=(const ParquetColumnChunkFileReader&) = delete; + + // Allow moving + ParquetColumnChunkFileReader(ParquetColumnChunkFileReader&&) noexcept; + ParquetColumnChunkFileReader& operator=(ParquetColumnChunkFileReader&&) noexcept; + + // FileReader interface implementation + Status close() override; + // const Path& path() const override { return path_; } + size_t size() const override { return chunks_.back()->file_size(); } + // bool closed() const override { return current_slice_ == nullptr; } + + // Additional functionality + Status addChunkReader(std::shared_ptr<ChunkReader> reader); + Status getSlice(const io::IOContext* io_ctx, size_t length, Slice* result); + size_t available() const { + return current_slice_ ? current_slice_->size - current_position_ : 0; + } + + // Status close() override { + // if (!_closed) { + // _closed = true; + // } + // return Status::OK(); + // } + + const io::Path& path() const override { return chunks_.back()->path(); } + + bool closed() const override { return _closed.load(std::memory_order_acquire); } + +protected: + Status read_at_impl(size_t offset, Slice result, size_t* bytes_read, + const io::IOContext* io_ctx) override; + + void _collect_profile_before_close() override { + // if (_profile != nullptr) { + // COUNTER_UPDATE(_copy_time, _statistics.copy_time); + // COUNTER_UPDATE(_read_time, _statistics.read_time); + // COUNTER_UPDATE(_request_io, _statistics.request_io); + // COUNTER_UPDATE(_merged_io, _statistics.merged_io); + // COUNTER_UPDATE(_request_bytes, _statistics.request_bytes); + // COUNTER_UPDATE(_merged_bytes, _statistics.merged_bytes); + // COUNTER_UPDATE(_apply_bytes, _statistics.apply_bytes); + // if (_reader != nullptr) { + // _reader->collect_profile_before_close(); + // } + // } + } + +private: + Status ensureOpen(); + Status readNextChunk(const io::IOContext* io_ctx); + +private: Review Comment: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers] ```suggestion ``` <details> <summary>Additional context</summary> **be/src/vec/exec/format/parquet/parquet_column_chunk_file_reader.h:96:** previously declared here ```cpp private: ^ ``` </details> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org
