Skip to content

Commit 9f54887

Browse files
committed
refactor(silo): styling improvements
1 parent 8d7b355 commit 9f54887

2 files changed

Lines changed: 31 additions & 29 deletions

File tree

src/silo/query_engine/exec_node/ndjson_sink.cpp

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "silo/query_engine/exec_node/ndjson_sink.h"
22

33
#include <ios>
4+
#include <string_view>
45

56
#include <arrow/acero/options.h>
67
#include <arrow/array.h>
@@ -10,6 +11,7 @@
1011
#include <nlohmann/json.hpp>
1112

1213
#include "evobench/evobench.hpp"
14+
#include "silo/common/panic.h"
1315
#include "silo/common/size_constants.h"
1416

1517
namespace silo::query_engine::exec_node {
@@ -26,35 +28,36 @@ void writeChunked(std::ostream& output, std::string_view content) {
2628
}
2729
}
2830

29-
template <size_t BATCH_SIZE>
31+
template <size_t BatchSize>
3032
struct BatchedStringStream {
31-
std::array<std::stringstream, BATCH_SIZE> streams;
33+
std::array<std::stringstream, BatchSize> streams;
3234

3335
void operator<<(std::string_view bytes) {
34-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
36+
for (size_t i = 0; i < BatchSize; ++i) {
3537
streams[i] << bytes;
3638
}
3739
}
3840

3941
void operator>>(std::ostream& output) {
40-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
41-
writeChunked(output, std::move(streams[i]).str());
42+
for (size_t i = 0; i < BatchSize; ++i) {
43+
std::string content = std::move(streams[i]).str();
44+
writeChunked(output, content);
4245
}
4346
}
4447
};
4548

46-
template <size_t BATCH_SIZE>
49+
template <size_t BatchSize>
4750
class ArrayToJsonTypeVisitor : public arrow::ArrayVisitor {
48-
BatchedStringStream<BATCH_SIZE>& output_stream;
51+
BatchedStringStream<BatchSize>& output_stream;
4952
size_t& row_base;
5053

5154
public:
52-
ArrayToJsonTypeVisitor(BatchedStringStream<BATCH_SIZE>& output_stream, size_t& row_base)
55+
ArrayToJsonTypeVisitor(BatchedStringStream<BatchSize>& output_stream, size_t& row_base)
5356
: output_stream(output_stream),
5457
row_base(row_base) {}
5558

5659
arrow::Status Visit(const arrow::Int32Array& array) override {
57-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
60+
for (size_t i = 0; i < BatchSize; ++i) {
5861
if (array.IsNull(row_base + i)) {
5962
output_stream.streams.at(i) << "null";
6063
} else {
@@ -65,7 +68,7 @@ class ArrayToJsonTypeVisitor : public arrow::ArrayVisitor {
6568
}
6669

6770
arrow::Status Visit(const arrow::Int64Array& array) override {
68-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
71+
for (size_t i = 0; i < BatchSize; ++i) {
6972
if (array.IsNull(row_base + i)) {
7073
output_stream.streams.at(i) << "null";
7174
} else {
@@ -76,31 +79,31 @@ class ArrayToJsonTypeVisitor : public arrow::ArrayVisitor {
7679
}
7780

7881
arrow::Status Visit(const arrow::DoubleArray& array) override {
79-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
82+
for (size_t i = 0; i < BatchSize; ++i) {
8083
if (array.IsNull(row_base + i)) {
8184
output_stream.streams.at(i) << "null";
8285
} else {
83-
nlohmann::json j = array.GetView(row_base + i);
84-
output_stream.streams.at(i) << j;
86+
nlohmann::json json = array.GetView(row_base + i);
87+
output_stream.streams.at(i) << json;
8588
}
8689
}
8790
return arrow::Status::OK();
8891
}
8992

9093
arrow::Status Visit(const arrow::FloatArray& array) override {
91-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
94+
for (size_t i = 0; i < BatchSize; ++i) {
9295
if (array.IsNull(row_base + i)) {
9396
output_stream.streams.at(i) << "null";
9497
} else {
95-
nlohmann::json j = array.GetView(row_base + i);
96-
output_stream.streams.at(i) << j;
98+
nlohmann::json json = array.GetView(row_base + i);
99+
output_stream.streams.at(i) << json;
97100
}
98101
}
99102
return arrow::Status::OK();
100103
}
101104

102105
arrow::Status Visit(const arrow::StringArray& array) override {
103-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
106+
for (size_t i = 0; i < BatchSize; ++i) {
104107
if (array.IsNull(row_base + i)) {
105108
output_stream.streams.at(i) << "null";
106109
} else {
@@ -113,7 +116,7 @@ class ArrayToJsonTypeVisitor : public arrow::ArrayVisitor {
113116
}
114117

115118
arrow::Status Visit(const arrow::BooleanArray& array) override {
116-
for (size_t i = 0; i < BATCH_SIZE; ++i) {
119+
for (size_t i = 0; i < BatchSize; ++i) {
117120
if (array.IsNull(row_base + i)) {
118121
output_stream.streams.at(i) << "null";
119122
} else {
@@ -124,18 +127,18 @@ class ArrayToJsonTypeVisitor : public arrow::ArrayVisitor {
124127
}
125128
};
126129

127-
template <size_t BATCH_SIZE>
130+
template <size_t BatchSize>
128131
void sendJsonLinesInBatches(
129132
size_t& row_idx_base,
130133
size_t row_count,
131134
const std::vector<std::string>& prepared_column_strings_for_json_attributes,
132135
const std::vector<std::shared_ptr<arrow::Array>>& column_arrays,
133136
std::ostream& output_stream
134137
) {
135-
BatchedStringStream<BATCH_SIZE> ndjson_line_streams;
136-
ArrayToJsonTypeVisitor<BATCH_SIZE> my_visitor(ndjson_line_streams, row_idx_base);
138+
BatchedStringStream<BatchSize> ndjson_line_streams;
139+
ArrayToJsonTypeVisitor<BatchSize> my_visitor(ndjson_line_streams, row_idx_base);
137140
size_t column_count = column_arrays.size();
138-
for (; row_idx_base + BATCH_SIZE <= row_count; row_idx_base += BATCH_SIZE) {
141+
for (; row_idx_base + BatchSize <= row_count; row_idx_base += BatchSize) {
139142
ndjson_line_streams << "{";
140143
for (size_t column_idx = 0; column_idx < column_count; column_idx++) {
141144
const auto& column_array = column_arrays.at(column_idx);
@@ -149,7 +152,7 @@ void sendJsonLinesInBatches(
149152
ndjson_line_streams >> output_stream;
150153
}
151154
}
152-
if constexpr (BATCH_SIZE > 1) {
155+
if constexpr (BatchSize > 1) {
153156
// Send remaining lines
154157
sendJsonLinesInBatches<1>(
155158
row_idx_base,
@@ -164,7 +167,7 @@ void sendJsonLinesInBatches(
164167
} // namespace
165168

166169
arrow::Status writeBatchAsNdjson(
167-
arrow::compute::ExecBatch batch,
170+
const arrow::compute::ExecBatch& batch,
168171
const std::shared_ptr<arrow::Schema>& schema,
169172
std::ostream* output_stream
170173
) {
@@ -180,8 +183,9 @@ arrow::Status writeBatchAsNdjson(
180183
for (const auto& column_name : schema->fields()) {
181184
nlohmann::json column_name_json = column_name->name();
182185
std::string json_formatted_column_name;
183-
if (!first_column)
186+
if (!first_column) {
184187
json_formatted_column_name += ",";
188+
}
185189
first_column = false;
186190
json_formatted_column_name += column_name_json.dump();
187191
json_formatted_column_name += ":";
@@ -217,7 +221,7 @@ arrow::Result<arrow::acero::BackpressureMonitor*> createGenerator(
217221
auto node, arrow::acero::MakeExecNode(std::string{"sink"}, plan, {input}, options)
218222
);
219223
node->SetLabel("final sink of the plan");
220-
return arrow::Result<arrow::acero::BackpressureMonitor*>{backpressure_monitor};
224+
return backpressure_monitor;
221225
}
222226

223227
} // namespace silo::query_engine::exec_node

src/silo/query_engine/exec_node/ndjson_sink.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@
66
#include <arrow/util/async_generator_fwd.h>
77
#include <spdlog/spdlog.h>
88

9-
#include "silo/common/panic.h"
10-
119
namespace silo::query_engine::exec_node {
1210

1311
arrow::Status writeBatchAsNdjson(
14-
arrow::compute::ExecBatch batch,
12+
const arrow::compute::ExecBatch& batch,
1513
const std::shared_ptr<arrow::Schema>& schema,
1614
std::ostream* output_stream
1715
);

0 commit comments

Comments
 (0)