jt2594838 commented on code in PR #747:
URL: https://github.com/apache/tsfile/pull/747#discussion_r2980224279


##########
cpp/src/common/tablet.h:
##########
@@ -162,6 +162,28 @@ class Tablet {
         err_code_ = init();
     }
 
+    /**
+     * @brief Constructs a Tablet object by taking pre-filled columns.
+     *
+     * For numeric types, `columns[col_index]` should point to a contiguous
+     * array of the corresponding C++ type.
+     * For string-like types (TEXT/BLOB/STRING), `columns[col_index]` should
+     * point to an array of `common::String` with length `row_num`.
+     */
+    Tablet(uint32_t column_num, uint32_t row_num, void** columns,
+           const std::vector<std::string>& column_names,
+           const std::vector<common::TSDataType>& data_types);
+
+    /**
+     * @brief Set a whole column buffer at once.
+     *
+     * @param column_index Schema index of the column to set.
+     * @param column A pointer to an array with `column_length` elements.
+     *               See the constructor doc for the expected element types.
+     * @param column_length Number of rows to copy from `column`.
+     */
+    int set_column(int column_index, void* column, int column_length);
+

Review Comment:
   How about the time column.



##########
cpp/src/common/tablet.cc:
##########
@@ -28,6 +29,112 @@ using namespace common;
 
 namespace storage {
 
+Tablet::Tablet(uint32_t column_num, uint32_t row_num, void** columns,
+               const std::vector<std::string>& column_names,
+               const std::vector<common::TSDataType>& data_types)
+    : Tablet(column_names, data_types, row_num) {
+    if (columns == nullptr) {
+        err_code_ = common::E_INVALID_ARG;
+        return;
+    }
+    ASSERT(column_num == column_names.size());
+    ASSERT(column_num == data_types.size());
+
+    for (uint32_t c = 0; c < column_num; ++c) {
+        int ret = set_column(static_cast<int>(c), columns[c],
+                             static_cast<int>(row_num));
+        if (ret != common::E_OK) {
+            err_code_ = ret;
+            break;
+        }
+    }
+}
+
+int Tablet::set_column(int column_index, void* column, int column_length) {
+    if (err_code_ != common::E_OK) {
+        return err_code_;
+    }
+    if (UNLIKELY(column_index < 0 ||
+                 static_cast<uint32_t>(column_index) >= schema_vec_->size())) {
+        return common::E_OUT_OF_RANGE;
+    }
+    if (UNLIKELY(column_length < 0)) {
+        return common::E_INVALID_ARG;
+    }
+    uint32_t len = static_cast<uint32_t>(column_length);
+    if (UNLIKELY(len > max_row_num_)) {
+        return common::E_OUT_OF_RANGE;
+    }
+    if (len == 0) {
+        return common::E_OK;
+    }
+
+    const MeasurementSchema& schema = schema_vec_->at(column_index);
+    auto& col_values = value_matrix_[column_index];
+    BitMap& col_notnull_bitmap = bitmaps_[column_index];
+
+    if (column == nullptr) {
+        // Treat as all-null.
+        for (uint32_t r = 0; r < len; ++r) {
+            col_notnull_bitmap.set(r);
+        }
+        cur_row_size_ = std::max(cur_row_size_, len);
+        return common::E_OK;

Review Comment:
   It seems that the bitmap is a null_bitmap.



##########
cpp/src/common/tablet.cc:
##########
@@ -28,6 +29,112 @@ using namespace common;
 
 namespace storage {
 
+Tablet::Tablet(uint32_t column_num, uint32_t row_num, void** columns,
+               const std::vector<std::string>& column_names,
+               const std::vector<common::TSDataType>& data_types)
+    : Tablet(column_names, data_types, row_num) {
+    if (columns == nullptr) {
+        err_code_ = common::E_INVALID_ARG;
+        return;
+    }
+    ASSERT(column_num == column_names.size());
+    ASSERT(column_num == data_types.size());
+
+    for (uint32_t c = 0; c < column_num; ++c) {
+        int ret = set_column(static_cast<int>(c), columns[c],
+                             static_cast<int>(row_num));
+        if (ret != common::E_OK) {
+            err_code_ = ret;
+            break;
+        }
+    }
+}
+
+int Tablet::set_column(int column_index, void* column, int column_length) {
+    if (err_code_ != common::E_OK) {
+        return err_code_;
+    }
+    if (UNLIKELY(column_index < 0 ||
+                 static_cast<uint32_t>(column_index) >= schema_vec_->size())) {
+        return common::E_OUT_OF_RANGE;
+    }
+    if (UNLIKELY(column_length < 0)) {
+        return common::E_INVALID_ARG;
+    }
+    uint32_t len = static_cast<uint32_t>(column_length);
+    if (UNLIKELY(len > max_row_num_)) {
+        return common::E_OUT_OF_RANGE;
+    }
+    if (len == 0) {
+        return common::E_OK;
+    }
+
+    const MeasurementSchema& schema = schema_vec_->at(column_index);
+    auto& col_values = value_matrix_[column_index];
+    BitMap& col_notnull_bitmap = bitmaps_[column_index];
+
+    if (column == nullptr) {
+        // Treat as all-null.
+        for (uint32_t r = 0; r < len; ++r) {
+            col_notnull_bitmap.set(r);
+        }
+        cur_row_size_ = std::max(cur_row_size_, len);
+        return common::E_OK;
+    }
+
+    switch (schema.data_type_) {
+        case common::BOOLEAN:
+            memcpy(col_values.bool_data, column, sizeof(bool) * len);
+            for (uint32_t r = 0; r < len; ++r) {
+                col_notnull_bitmap.clear(r);
+            }
+            break;

Review Comment:
   Need to directly assign the pointer instead of a memory copy.
   
   The method should be extremely fast but may be unsafe.
   
   



##########
cpp/src/common/tablet.h:
##########
@@ -162,6 +162,28 @@ class Tablet {
         err_code_ = init();
     }
 
+    /**
+     * @brief Constructs a Tablet object by taking pre-filled columns.
+     *
+     * For numeric types, `columns[col_index]` should point to a contiguous
+     * array of the corresponding C++ type.
+     * For string-like types (TEXT/BLOB/STRING), `columns[col_index]` should
+     * point to an array of `common::String` with length `row_num`.
+     */
+    Tablet(uint32_t column_num, uint32_t row_num, void** columns,
+           const std::vector<std::string>& column_names,
+           const std::vector<common::TSDataType>& data_types);
+
+    /**
+     * @brief Set a whole column buffer at once.
+     *

Review Comment:
   Mention how cur_row_size is updated after calling this method.



-- 
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]

Reply via email to