You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "charlesdong1991 (via GitHub)" <gi...@apache.org> on 2026/04/18 11:22:25 UTC
[PR] [c++] Add c++ support for Array data type [fluss-rust]
charlesdong1991 opened a new pull request, #501:
URL: https://github.com/apache/fluss-rust/pull/501
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #468
<!-- What is the purpose of the change -->
### Brief change log
Adds ARRAY data type support to the C++ bindings
### Tests
All tests are passed
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3105873161
##########
bindings/cpp/src/table.cpp:
##########
@@ -191,6 +404,14 @@ void GenericRow::SetDecimal(size_t idx, const std::string& value) {
inner_->gr_set_decimal_str(idx, value);
}
+void GenericRow::SetArray(size_t idx, ArrayWriter&& writer) {
Review Comment:
Similarly here on calling `Destroy()`
##########
bindings/cpp/src/table.cpp:
##########
@@ -85,6 +85,219 @@ int Date::Day() const {
if (!inner_) throw std::logic_error(name ": not available (moved-from or null)"); \
} while (0)
+// ============================================================================
+// ArrayWriter — builder for array values backed by Rust ArrayWriterInner
+// ============================================================================
+
+ArrayWriter::ArrayWriter(size_t size, DataType element_type) : element_type_(std::move(element_type)) {
+ auto flat = utils::flatten_array_type(element_type_);
+ int32_t leaf_type_id = flat.nesting > 0 ? flat.leaf_type : static_cast<int32_t>(element_type_.id());
+ uint32_t leaf_precision = static_cast<uint32_t>(flat.nesting > 0 ? flat.leaf_precision
+ : element_type_.precision());
+ uint32_t leaf_scale = static_cast<uint32_t>(flat.nesting > 0 ? flat.leaf_scale : element_type_.scale());
+ uint32_t array_nesting = static_cast<uint32_t>(flat.nesting);
+
+ auto box = ffi::new_array_writer(size, leaf_type_id, leaf_precision, leaf_scale, array_nesting);
+ inner_ = box.into_raw();
+}
+
+ArrayWriter::~ArrayWriter() noexcept { Destroy(); }
+
+void ArrayWriter::Destroy() noexcept {
+ if (inner_) {
+ rust::Box<ffi::ArrayWriterInner>::from_raw(inner_);
+ inner_ = nullptr;
+ }
+}
+
+ArrayWriter::ArrayWriter(ArrayWriter&& other) noexcept
+ : inner_(other.inner_), element_type_(std::move(other.element_type_)) {
+ other.inner_ = nullptr;
+}
+
+ArrayWriter& ArrayWriter::operator=(ArrayWriter&& other) noexcept {
+ if (this != &other) {
+ Destroy();
+ inner_ = other.inner_;
+ element_type_ = std::move(other.element_type_);
+ other.inner_ = nullptr;
+ }
+ return *this;
+}
+
+bool ArrayWriter::Available() const { return inner_ != nullptr; }
+
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
+#define CHECK_AW(name) \
+ do { \
+ if (!inner_) throw std::logic_error(name ": not available (moved-from or null)"); \
+ } while (0)
+
+void ArrayWriter::SetNull(size_t idx) { CHECK_AW("ArrayWriter"); inner_->aw_set_null(idx); }
+void ArrayWriter::SetBool(size_t idx, bool v) { CHECK_AW("ArrayWriter"); inner_->aw_set_bool(idx, v); }
+void ArrayWriter::SetInt32(size_t idx, int32_t v) { CHECK_AW("ArrayWriter"); inner_->aw_set_i32(idx, v); }
+void ArrayWriter::SetInt64(size_t idx, int64_t v) { CHECK_AW("ArrayWriter"); inner_->aw_set_i64(idx, v); }
+void ArrayWriter::SetFloat32(size_t idx, float v) { CHECK_AW("ArrayWriter"); inner_->aw_set_f32(idx, v); }
+void ArrayWriter::SetFloat64(size_t idx, double v) { CHECK_AW("ArrayWriter"); inner_->aw_set_f64(idx, v); }
+
+void ArrayWriter::SetString(size_t idx, const std::string& v) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_str(idx, v);
+}
+
+void ArrayWriter::SetBytes(size_t idx, const std::vector<uint8_t>& v) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_bytes(idx, rust::Slice<const uint8_t>(v.data(), v.size()));
+}
+
+void ArrayWriter::SetDate(size_t idx, fluss::Date d) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_date(idx, d.days_since_epoch);
+}
+
+void ArrayWriter::SetTime(size_t idx, fluss::Time t) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_time(idx, t.millis_since_midnight);
+}
+
+void ArrayWriter::SetTimestampNtz(size_t idx, fluss::Timestamp ts) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_ts_ntz(idx, ts.epoch_millis, ts.nano_of_millisecond);
+}
+
+void ArrayWriter::SetTimestampLtz(size_t idx, fluss::Timestamp ts) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_ts_ltz(idx, ts.epoch_millis, ts.nano_of_millisecond);
+}
+
+void ArrayWriter::SetDecimal(size_t idx, const std::string& value) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_decimal_str(idx, value);
+}
+
+void ArrayWriter::SetArray(size_t idx, ArrayWriter&& nested) {
+ CHECK_AW("ArrayWriter");
+ if (!nested.inner_) {
+ throw std::logic_error("ArrayWriter::SetArray: nested writer not available");
+ }
+ inner_->aw_set_array(idx, *nested.inner_);
+}
Review Comment:
Not familiar with C++, but it seems like the convention of `left::method(right&& arg)` is for left to take ownership.
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#rf-consume
Should we call `Destroy()` to ensure no memory leak?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3119868131
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -535,6 +603,90 @@ struct ScanData {
};
} // namespace detail
+/**
+ * @brief Read-only view over a FlussArray column value.
+ *
+ * Obtained from RowView::GetArrayView() / LookupResult::GetArrayView(), and
+ * recursively from ArrayView::GetArray() for nested `ARRAY<ARRAY<...>>`
+ * columns. Owns an opaque Rust handle (FlussArray + element DataType) that
+ * is released on destruction. Move-only.
+ */
+class ArrayView {
+ public:
+ ~ArrayView() noexcept;
+
+ ArrayView(const ArrayView&) = delete;
+ ArrayView& operator=(const ArrayView&) = delete;
+ ArrayView(ArrayView&& other) noexcept;
+ ArrayView& operator=(ArrayView&& other) noexcept;
+
+ size_t Size() const;
+ TypeId ElementType() const;
+ bool IsNull(size_t element) const;
+
+ bool GetBool(size_t element) const;
+ int32_t GetInt32(size_t element) const;
+ int64_t GetInt64(size_t element) const;
+ float GetFloat32(size_t element) const;
+ double GetFloat64(size_t element) const;
+ std::string GetString(size_t element) const;
+ std::vector<uint8_t> GetBytes(size_t element) const;
+ fluss::Date GetDate(size_t element) const;
+ fluss::Time GetTime(size_t element) const;
+ fluss::Timestamp GetTimestampNtz(size_t element) const;
+ fluss::Timestamp GetTimestampLtz(size_t element) const;
+ std::string GetDecimalString(size_t element) const;
+ ArrayView GetArray(size_t element) const;
+
+ private:
+ friend class RowView;
+ friend class LookupResult;
+ explicit ArrayView(ffi::ArrayViewInner* inner) : inner_(inner) {}
+ void Destroy() noexcept;
+ ffi::ArrayViewInner* inner_{nullptr};
+};
+
+namespace detail {
+template <typename Derived>
+inline ArrayView NamedGetters<Derived>::GetArrayView(const std::string& n) const {
+ return Self().GetArrayView(Self().Resolve(n));
+}
+} // namespace detail
+
+class ArrayWriter {
Review Comment:
done, i also expose Size to arraywriter for symmetry with arrayview
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia merged PR #501:
URL: https://github.com/apache/fluss-rust/pull/501
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3126569130
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -305,15 +308,29 @@ class DataType {
}
static DataType Char(int32_t length) { return DataType(TypeId::Char, length, 0); }
static DataType Binary(int32_t length) { return DataType(TypeId::Binary, length, 0); }
+ /// Constructs an `ARRAY<element>` type. The element DataType (possibly
+ /// itself an array) is deep-copied into a shared owning handle so that
+ /// copies of the outer DataType remain cheap while the element lives
+ /// as long as any reference exists.
+ static DataType Array(DataType element) {
Review Comment:
Created 2 issues
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3106592914
##########
bindings/cpp/src/table.cpp:
##########
@@ -191,6 +404,14 @@ void GenericRow::SetDecimal(size_t idx, const std::string& value) {
inner_->gr_set_decimal_str(idx, value);
}
+void GenericRow::SetArray(size_t idx, ArrayWriter&& writer) {
Review Comment:
the same as above
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3122033164
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -305,15 +308,29 @@ class DataType {
}
static DataType Char(int32_t length) { return DataType(TypeId::Char, length, 0); }
static DataType Binary(int32_t length) { return DataType(TypeId::Binary, length, 0); }
+ /// Constructs an `ARRAY<element>` type. The element DataType (possibly
+ /// itself an array) is deep-copied into a shared owning handle so that
+ /// copies of the outer DataType remain cheap while the element lives
+ /// as long as any reference exists.
+ static DataType Array(DataType element) {
Review Comment:
Sure, it works
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3119879937
##########
bindings/cpp/test/test_log_table.cpp:
##########
@@ -829,3 +829,436 @@ TEST_F(LogTableTest, PartitionedTableAppendScan) {
ASSERT_OK(adm.DropTable(table_path, false));
}
+
+// ============================================================================
+// Array data type tests
Review Comment:
nice!
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3119805314
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -305,15 +308,29 @@ class DataType {
}
static DataType Char(int32_t length) { return DataType(TypeId::Char, length, 0); }
static DataType Binary(int32_t length) { return DataType(TypeId::Binary, length, 0); }
+ /// Constructs an `ARRAY<element>` type. The element DataType (possibly
+ /// itself an array) is deep-copied into a shared owning handle so that
+ /// copies of the outer DataType remain cheap while the element lives
+ /// as long as any reference exists.
+ static DataType Array(DataType element) {
+ DataType dt(TypeId::Array, 0, 0);
+ dt.element_type_ = std::make_shared<DataType>(std::move(element));
+ return dt;
+ }
TypeId id() const { return id_; }
int32_t precision() const { return precision_; }
int32_t scale() const { return scale_; }
+ /// Returns the element type of an ARRAY. Returns `nullptr` for non-array
+ /// types. The returned pointer is valid as long as this DataType (or a
+ /// copy holding the same shared element) is alive.
+ const DataType* element_type() const { return element_type_.get(); }
private:
TypeId id_;
int32_t precision_{0};
int32_t scale_{0};
+ std::shared_ptr<DataType> element_type_;
Review Comment:
same above
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#issuecomment-4273703771
@fresh-borzoni @leekeiabstraction appreciate it a lot if you can take a look! 🙏 thanks!
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3119881789
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -535,6 +603,90 @@ struct ScanData {
};
} // namespace detail
+/**
+ * @brief Read-only view over a FlussArray column value.
+ *
+ * Obtained from RowView::GetArrayView() / LookupResult::GetArrayView(), and
+ * recursively from ArrayView::GetArray() for nested `ARRAY<ARRAY<...>>`
+ * columns. Owns an opaque Rust handle (FlussArray + element DataType) that
+ * is released on destruction. Move-only.
+ */
+class ArrayView {
+ public:
+ ~ArrayView() noexcept;
+
+ ArrayView(const ArrayView&) = delete;
+ ArrayView& operator=(const ArrayView&) = delete;
+ ArrayView(ArrayView&& other) noexcept;
+ ArrayView& operator=(ArrayView&& other) noexcept;
+
+ size_t Size() const;
Review Comment:
changed!
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3106591168
##########
bindings/cpp/src/table.cpp:
##########
@@ -85,6 +85,219 @@ int Date::Day() const {
if (!inner_) throw std::logic_error(name ": not available (moved-from or null)"); \
} while (0)
+// ============================================================================
+// ArrayWriter — builder for array values backed by Rust ArrayWriterInner
+// ============================================================================
+
+ArrayWriter::ArrayWriter(size_t size, DataType element_type) : element_type_(std::move(element_type)) {
+ auto flat = utils::flatten_array_type(element_type_);
+ int32_t leaf_type_id = flat.nesting > 0 ? flat.leaf_type : static_cast<int32_t>(element_type_.id());
+ uint32_t leaf_precision = static_cast<uint32_t>(flat.nesting > 0 ? flat.leaf_precision
+ : element_type_.precision());
+ uint32_t leaf_scale = static_cast<uint32_t>(flat.nesting > 0 ? flat.leaf_scale : element_type_.scale());
+ uint32_t array_nesting = static_cast<uint32_t>(flat.nesting);
+
+ auto box = ffi::new_array_writer(size, leaf_type_id, leaf_precision, leaf_scale, array_nesting);
+ inner_ = box.into_raw();
+}
+
+ArrayWriter::~ArrayWriter() noexcept { Destroy(); }
+
+void ArrayWriter::Destroy() noexcept {
+ if (inner_) {
+ rust::Box<ffi::ArrayWriterInner>::from_raw(inner_);
+ inner_ = nullptr;
+ }
+}
+
+ArrayWriter::ArrayWriter(ArrayWriter&& other) noexcept
+ : inner_(other.inner_), element_type_(std::move(other.element_type_)) {
+ other.inner_ = nullptr;
+}
+
+ArrayWriter& ArrayWriter::operator=(ArrayWriter&& other) noexcept {
+ if (this != &other) {
+ Destroy();
+ inner_ = other.inner_;
+ element_type_ = std::move(other.element_type_);
+ other.inner_ = nullptr;
+ }
+ return *this;
+}
+
+bool ArrayWriter::Available() const { return inner_ != nullptr; }
+
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
+#define CHECK_AW(name) \
+ do { \
+ if (!inner_) throw std::logic_error(name ": not available (moved-from or null)"); \
+ } while (0)
+
+void ArrayWriter::SetNull(size_t idx) { CHECK_AW("ArrayWriter"); inner_->aw_set_null(idx); }
+void ArrayWriter::SetBool(size_t idx, bool v) { CHECK_AW("ArrayWriter"); inner_->aw_set_bool(idx, v); }
+void ArrayWriter::SetInt32(size_t idx, int32_t v) { CHECK_AW("ArrayWriter"); inner_->aw_set_i32(idx, v); }
+void ArrayWriter::SetInt64(size_t idx, int64_t v) { CHECK_AW("ArrayWriter"); inner_->aw_set_i64(idx, v); }
+void ArrayWriter::SetFloat32(size_t idx, float v) { CHECK_AW("ArrayWriter"); inner_->aw_set_f32(idx, v); }
+void ArrayWriter::SetFloat64(size_t idx, double v) { CHECK_AW("ArrayWriter"); inner_->aw_set_f64(idx, v); }
+
+void ArrayWriter::SetString(size_t idx, const std::string& v) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_str(idx, v);
+}
+
+void ArrayWriter::SetBytes(size_t idx, const std::vector<uint8_t>& v) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_bytes(idx, rust::Slice<const uint8_t>(v.data(), v.size()));
+}
+
+void ArrayWriter::SetDate(size_t idx, fluss::Date d) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_date(idx, d.days_since_epoch);
+}
+
+void ArrayWriter::SetTime(size_t idx, fluss::Time t) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_time(idx, t.millis_since_midnight);
+}
+
+void ArrayWriter::SetTimestampNtz(size_t idx, fluss::Timestamp ts) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_ts_ntz(idx, ts.epoch_millis, ts.nano_of_millisecond);
+}
+
+void ArrayWriter::SetTimestampLtz(size_t idx, fluss::Timestamp ts) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_ts_ltz(idx, ts.epoch_millis, ts.nano_of_millisecond);
+}
+
+void ArrayWriter::SetDecimal(size_t idx, const std::string& value) {
+ CHECK_AW("ArrayWriter");
+ inner_->aw_set_decimal_str(idx, value);
+}
+
+void ArrayWriter::SetArray(size_t idx, ArrayWriter&& nested) {
+ CHECK_AW("ArrayWriter");
+ if (!nested.inner_) {
+ throw std::logic_error("ArrayWriter::SetArray: nested writer not available");
+ }
+ inner_->aw_set_array(idx, *nested.inner_);
+}
Review Comment:
i dont think it will cause memory leak since when nested goes out of scope at caller, ArrayWriter will run and call destroy, and will free `Box` from rust, so cleanup will happen.
But with another thought, i think it is still worth adding here explicitly, since it is aligned with c++ convention, let me update
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3107202022
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -535,6 +603,90 @@ struct ScanData {
};
} // namespace detail
+/**
+ * @brief Read-only view over a FlussArray column value.
+ *
+ * Obtained from RowView::GetArrayView() / LookupResult::GetArrayView(), and
+ * recursively from ArrayView::GetArray() for nested `ARRAY<ARRAY<...>>`
+ * columns. Owns an opaque Rust handle (FlussArray + element DataType) that
+ * is released on destruction. Move-only.
+ */
+class ArrayView {
+ public:
+ ~ArrayView() noexcept;
+
+ ArrayView(const ArrayView&) = delete;
+ ArrayView& operator=(const ArrayView&) = delete;
+ ArrayView(ArrayView&& other) noexcept;
+ ArrayView& operator=(ArrayView&& other) noexcept;
+
+ size_t Size() const;
Review Comment:
noexcept for Size and ElementType?
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -535,6 +603,90 @@ struct ScanData {
};
} // namespace detail
+/**
+ * @brief Read-only view over a FlussArray column value.
+ *
+ * Obtained from RowView::GetArrayView() / LookupResult::GetArrayView(), and
+ * recursively from ArrayView::GetArray() for nested `ARRAY<ARRAY<...>>`
+ * columns. Owns an opaque Rust handle (FlussArray + element DataType) that
+ * is released on destruction. Move-only.
+ */
+class ArrayView {
+ public:
+ ~ArrayView() noexcept;
+
+ ArrayView(const ArrayView&) = delete;
+ ArrayView& operator=(const ArrayView&) = delete;
+ ArrayView(ArrayView&& other) noexcept;
+ ArrayView& operator=(ArrayView&& other) noexcept;
+
+ size_t Size() const;
+ TypeId ElementType() const;
+ bool IsNull(size_t element) const;
+
+ bool GetBool(size_t element) const;
+ int32_t GetInt32(size_t element) const;
+ int64_t GetInt64(size_t element) const;
+ float GetFloat32(size_t element) const;
+ double GetFloat64(size_t element) const;
+ std::string GetString(size_t element) const;
+ std::vector<uint8_t> GetBytes(size_t element) const;
+ fluss::Date GetDate(size_t element) const;
+ fluss::Time GetTime(size_t element) const;
+ fluss::Timestamp GetTimestampNtz(size_t element) const;
+ fluss::Timestamp GetTimestampLtz(size_t element) const;
+ std::string GetDecimalString(size_t element) const;
+ ArrayView GetArray(size_t element) const;
+
+ private:
+ friend class RowView;
+ friend class LookupResult;
+ explicit ArrayView(ffi::ArrayViewInner* inner) : inner_(inner) {}
+ void Destroy() noexcept;
+ ffi::ArrayViewInner* inner_{nullptr};
+};
+
+namespace detail {
+template <typename Derived>
+inline ArrayView NamedGetters<Derived>::GetArrayView(const std::string& n) const {
+ return Self().GetArrayView(Self().Resolve(n));
+}
+} // namespace detail
+
+class ArrayWriter {
Review Comment:
we have num elements as well if I recall correctly
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -305,15 +308,29 @@ class DataType {
}
static DataType Char(int32_t length) { return DataType(TypeId::Char, length, 0); }
static DataType Binary(int32_t length) { return DataType(TypeId::Binary, length, 0); }
+ /// Constructs an `ARRAY<element>` type. The element DataType (possibly
+ /// itself an array) is deep-copied into a shared owning handle so that
+ /// copies of the outer DataType remain cheap while the element lives
+ /// as long as any reference exists.
+ static DataType Array(DataType element) {
+ DataType dt(TypeId::Array, 0, 0);
+ dt.element_type_ = std::make_shared<DataType>(std::move(element));
+ return dt;
+ }
TypeId id() const { return id_; }
int32_t precision() const { return precision_; }
int32_t scale() const { return scale_; }
+ /// Returns the element type of an ARRAY. Returns `nullptr` for non-array
+ /// types. The returned pointer is valid as long as this DataType (or a
+ /// copy holding the same shared element) is alive.
+ const DataType* element_type() const { return element_type_.get(); }
private:
TypeId id_;
int32_t precision_{0};
int32_t scale_{0};
+ std::shared_ptr<DataType> element_type_;
Review Comment:
also is_nullable field needed here
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -305,15 +308,29 @@ class DataType {
}
static DataType Char(int32_t length) { return DataType(TypeId::Char, length, 0); }
static DataType Binary(int32_t length) { return DataType(TypeId::Binary, length, 0); }
+ /// Constructs an `ARRAY<element>` type. The element DataType (possibly
+ /// itself an array) is deep-copied into a shared owning handle so that
+ /// copies of the outer DataType remain cheap while the element lives
+ /// as long as any reference exists.
+ static DataType Array(DataType element) {
Review Comment:
Should we add element nullability?
Rust's ArrayType::with_nullable(false, e) carries it, by dropping here we silently lose ARRAY<X NOT NULL> on schema round-trip. Probably the same applies to python, do you mind to check?
##########
bindings/cpp/src/ffi_converter.hpp:
##########
@@ -20,13 +20,72 @@
#pragma once
#include <cassert>
+#include <stdexcept>
#include "fluss.hpp"
#include "lib.rs.h"
namespace fluss {
namespace utils {
+/// Compact FFI representation of a (possibly nested) array type.
+///
+/// `nesting` counts the number of ARRAY wrappers stripped to reach the leaf
+/// element type. `leaf_type`/`leaf_precision`/`leaf_scale` describe that leaf
+/// scalar. A non-array input produces a zero-initialised value (nesting == 0).
+///
+/// Using a flat representation — rather than serialising a recursive
+/// `DataType` — keeps the cxx bridge contract small (four `i32`s inside
+/// `FfiColumn`) while preserving full schema fidelity across the FFI boundary
+/// when paired with rebuild_array_type().
+struct FlattenedArrayType {
+ int32_t nesting{0};
+ int32_t leaf_type{0};
+ int32_t leaf_precision{0};
Review Comment:
also thread nullability here
##########
bindings/cpp/test/test_log_table.cpp:
##########
@@ -829,3 +829,436 @@ TEST_F(LogTableTest, PartitionedTableAppendScan) {
ASSERT_OK(adm.DropTable(table_path, false));
}
+
+// ============================================================================
+// Array data type tests
Review Comment:
I am thinking about these edge cases as well:
- Long string in array (≥8 bytes for variable-length encoding path)
- ARRAY<DECIMAL(20+, 5)> for non-compact decimal
- ARRAY<TIMESTAMP(9)> for non-compact timestamp
- Float NaN for ±Infinity round-trip
- ARRAY<BINARY(N)> for fixed-length
- SetInt32(1000) on TINYINT must throw
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [c++] Add c++ support for Array data type [fluss-rust]
Posted by "charlesdong1991 (via GitHub)" <gi...@apache.org>.
charlesdong1991 commented on code in PR #501:
URL: https://github.com/apache/fluss-rust/pull/501#discussion_r3119804281
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -305,15 +308,29 @@ class DataType {
}
static DataType Char(int32_t length) { return DataType(TypeId::Char, length, 0); }
static DataType Binary(int32_t length) { return DataType(TypeId::Binary, length, 0); }
+ /// Constructs an `ARRAY<element>` type. The element DataType (possibly
+ /// itself an array) is deep-copied into a shared owning handle so that
+ /// copies of the outer DataType remain cheap while the element lives
+ /// as long as any reference exists.
+ static DataType Array(DataType element) {
Review Comment:
Good observation, with another look, it seems nullability is actually not in place for every data type, not limited to Array. The same with python.
I suggest create a separate issue to fix nullability instead, and focus on supporting array type in this PR.
WDYT? If you agree, i can create 2 issues for python and c++ @fresh-borzoni
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org