github-actions[bot] commented on code in PR #36010:
URL: https://github.com/apache/doris/pull/36010#discussion_r1630543087


##########
be/src/pipeline/pipeline_x/local_exchange/local_exchanger.h:
##########
@@ -58,9 +65,60 @@
     const int _num_partitions;
     const int _num_senders;
     const int _num_sources;
+    const int _free_block_limit = 0;
     moodycamel::ConcurrentQueue<vectorized::Block> _free_blocks;
 };
 
+struct PartitionedRowIdxs {
+    std::shared_ptr<vectorized::PODArray<uint32_t>> row_idxs;
+    uint32_t offset_start;
+    uint32_t length;
+};
+
+using PartitionedBlock = std::pair<std::shared_ptr<ShuffleBlockWrapper>, 
PartitionedRowIdxs>;
+
+template <typename BlockType>
+struct BlockQueue {
+    std::atomic<bool> eos = false;
+    moodycamel::ConcurrentQueue<BlockType> data_queue;
+    BlockQueue() : eos(false), 
data_queue(moodycamel::ConcurrentQueue<BlockType>()) {}

Review Comment:
   warning: use '= default' to define a trivial default constructor 
[modernize-use-equals-default]
   
   ```suggestion
       BlockQueue() : eos(false), 
data_queue(moodycamel::ConcurrentQueue<BlockType>()) = default;
   ```
   



##########
be/src/pipeline/pipeline_x/local_exchange/local_exchanger.cpp:
##########
@@ -15,10 +15,14 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "pipeline/pipeline_x/local_exchange/local_exchanger.h"
+#include "pipeline/local_exchange/local_exchanger.h"

Review Comment:
   warning: 'pipeline/local_exchange/local_exchanger.h' file not found 
[clang-diagnostic-error]
   ```cpp
   #include "pipeline/local_exchange/local_exchanger.h"
            ^
   ```
   



##########
be/src/pipeline/pipeline_x/local_exchange/local_exchanger.h:
##########
@@ -17,35 +17,42 @@
 
 #pragma once
 
-#include "pipeline/pipeline_x/operator.h"
+#include "pipeline/dependency.h"

Review Comment:
   warning: 'pipeline/dependency.h' file not found [clang-diagnostic-error]
   ```cpp
   #include "pipeline/dependency.h"
            ^
   ```
   



##########
be/src/pipeline/pipeline_x/local_exchange/local_exchanger.h:
##########
@@ -58,9 +65,60 @@
     const int _num_partitions;
     const int _num_senders;
     const int _num_sources;
+    const int _free_block_limit = 0;
     moodycamel::ConcurrentQueue<vectorized::Block> _free_blocks;
 };
 
+struct PartitionedRowIdxs {
+    std::shared_ptr<vectorized::PODArray<uint32_t>> row_idxs;
+    uint32_t offset_start;
+    uint32_t length;
+};
+
+using PartitionedBlock = std::pair<std::shared_ptr<ShuffleBlockWrapper>, 
PartitionedRowIdxs>;
+
+template <typename BlockType>
+struct BlockQueue {
+    std::atomic<bool> eos = false;
+    moodycamel::ConcurrentQueue<BlockType> data_queue;
+    BlockQueue() : eos(false), 
data_queue(moodycamel::ConcurrentQueue<BlockType>()) {}
+    BlockQueue(BlockQueue<BlockType>&& other)
+            : eos(other.eos.load()), data_queue(std::move(other.data_queue)) {}
+    inline bool enqueue(BlockType const& item) {
+        if (!eos) {
+            data_queue.enqueue(item);
+            return true;
+        }
+        return false;
+    }
+
+    inline bool enqueue(BlockType&& item) {
+        if (!eos) {
+            data_queue.enqueue(std::move(item));
+            return true;

Review Comment:
   warning: redundant boolean literal in conditional return statement 
[readability-simplify-boolean-expr]
   
   be/src/pipeline/pipeline_x/local_exchange/local_exchanger.h:95:
   ```diff
   -         if (!eos) {
   -             data_queue.enqueue(std::move(item));
   -             return true;
   -         }
   -         return false;
   +         return !eos;
   ```
   



##########
be/src/pipeline/pipeline_x/local_exchange/local_exchanger.h:
##########
@@ -58,9 +65,60 @@
     const int _num_partitions;
     const int _num_senders;
     const int _num_sources;
+    const int _free_block_limit = 0;
     moodycamel::ConcurrentQueue<vectorized::Block> _free_blocks;
 };
 
+struct PartitionedRowIdxs {
+    std::shared_ptr<vectorized::PODArray<uint32_t>> row_idxs;
+    uint32_t offset_start;
+    uint32_t length;
+};
+
+using PartitionedBlock = std::pair<std::shared_ptr<ShuffleBlockWrapper>, 
PartitionedRowIdxs>;
+
+template <typename BlockType>
+struct BlockQueue {
+    std::atomic<bool> eos = false;
+    moodycamel::ConcurrentQueue<BlockType> data_queue;
+    BlockQueue() : eos(false), 
data_queue(moodycamel::ConcurrentQueue<BlockType>()) {}
+    BlockQueue(BlockQueue<BlockType>&& other)
+            : eos(other.eos.load()), data_queue(std::move(other.data_queue)) {}
+    inline bool enqueue(BlockType const& item) {
+        if (!eos) {
+            data_queue.enqueue(item);
+            return true;

Review Comment:
   warning: redundant boolean literal in conditional return statement 
[readability-simplify-boolean-expr]
   
   be/src/pipeline/pipeline_x/local_exchange/local_exchanger.h:87:
   ```diff
   -         if (!eos) {
   -             data_queue.enqueue(item);
   -             return true;
   -         }
   -         return false;
   +         return !eos;
   ```
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to