Add C++ and Python TsFile properties support - #897
Open
ColinLeeo wants to merge 3 commits into
Open
Conversation
ColinLeeo
requested review from
hongzhi-gao and
jt2594838
and
a lite review from Copilot
August 5, 2026 08:14
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds file-level TsFile property support across C++, the C wrapper, and Python APIs, enabling binary property read/write with null vs empty distinction and compatibility with Java’s Map<String, byte[]>.
Changes:
- Introduces
TsFilePropertyValue/TsFileProperties, wires property storage into writer/metadata serialization and reader access. - Extends C wrapper with length-aware setter and reader APIs plus explicit free function.
- Adds Python writer/reader bindings plus tests and documentation updates.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| python/tsfile/tsfile_writer.pyx | Adds Python writer API to set binary file properties via C wrapper. |
| python/tsfile/tsfile_table_writer.py | Delegates table-writer property setting to underlying writer. |
| python/tsfile/tsfile_reader.pyx | Adds Python reader API to retrieve properties as `dict[str, bytes |
| python/tsfile/tsfile_cpp.pxd | Declares new C wrapper structs/enums and functions for properties. |
| python/tests/test_tsfile_properties.py | Adds Python tests for round-trip, types, binary/NUL behavior, and closed-writer rejection. |
| python/README.md | Documents Python file-level properties API and encoding guidance. |
| python/README-zh.md | Chinese documentation for Python file-level properties. |
| cpp/test/writer/tsfile_properties_test.cc | Adds C++ tests for property semantics and system property precedence. |
| cpp/test/writer/table_view/tsfile_writer_table_test.cc | Adds table-writer delegation test for properties. |
| cpp/test/cwrapper/cwrapper_properties_test.cc | Adds C wrapper tests for length-aware round-trips, validation, and cleanup. |
| cpp/test/common/tsfile_common_test.cc | Updates metadata serialization test to new value-owned property model. |
| cpp/src/writer/tsfile_writer.h / .cc | Adds C++ writer public setters for properties. |
| cpp/src/writer/tsfile_table_writer.h / .cc | Adds C++ table-writer setters delegating to writer with closed checks. |
| cpp/src/reader/tsfile_reader.h / .cc | Adds C++ reader API to return file-level properties. |
| cpp/src/file/tsfile_io_writer.h / .cc | Stores properties in IO writer; serializes them into TsFileMeta, enforces system property override. |
| cpp/src/cwrapper/tsfile_cwrapper.h / .cc | Adds C APIs for setting/reading properties plus memory-free helper and validation. |
| cpp/src/common/tsfile_common.h / .cc | Introduces TsFilePropertyValue and updates meta (de)serialization format handling. |
| cpp/README.md | Documents C++ file-level properties usage. |
| cpp/README-zh.md | Chinese documentation for C++ file-level properties. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+265
to
+313
| if (RET_FAIL(common::SerializationUtil::read_var_int(tsfile_properties_size, | ||
| in))) { | ||
| return ret; | ||
| } | ||
| if (tsfile_properties_size < 0) { | ||
| return common::E_TSFILE_CORRUPTED; | ||
| } | ||
| for (int i = 0; i < tsfile_properties_size; i++) { | ||
| std::string key, *value; | ||
| common::SerializationUtil::read_var_str(key, in); | ||
| common::SerializationUtil::read_var_char_ptr(value, in); | ||
| tsfile_properties_.emplace(key, value); | ||
| std::string key; | ||
| int32_t key_len = 0; | ||
| int32_t value_len = 0; | ||
| if (RET_FAIL(common::SerializationUtil::read_var_int(key_len, in))) { | ||
| return ret; | ||
| } else if (key_len < 0) { | ||
| return common::E_TSFILE_CORRUPTED; | ||
| } | ||
| key.resize(static_cast<size_t>(key_len)); | ||
| if (key_len > 0) { | ||
| uint32_t read_len = 0; | ||
| if (RET_FAIL(in.read_buf(reinterpret_cast<uint8_t*>(&key[0]), | ||
| static_cast<uint32_t>(key_len), | ||
| read_len))) { | ||
| return ret; | ||
| } else if (read_len != static_cast<uint32_t>(key_len)) { | ||
| return common::E_BUF_NOT_ENOUGH; | ||
| } | ||
| } | ||
| if (RET_FAIL(common::SerializationUtil::read_var_int(value_len, in))) { | ||
| return ret; | ||
| } | ||
|
|
||
| TsFilePropertyValue value; | ||
| if (value_len == NO_STR_TO_READ) { | ||
| value.is_null = true; | ||
| } else if (value_len < 0) { | ||
| return common::E_TSFILE_CORRUPTED; | ||
| } else { | ||
| value.is_null = false; | ||
| value.value.resize(static_cast<size_t>(value_len)); | ||
| if (value_len > 0) { | ||
| uint32_t read_len = 0; | ||
| if (RET_FAIL( | ||
| in.read_buf(value.value.data(), value_len, read_len))) { | ||
| return ret; | ||
| } else if (read_len != static_cast<uint32_t>(value_len)) { | ||
| return common::E_BUF_NOT_ENOUGH; | ||
| } | ||
| } | ||
| } |
Comment on lines
+209
to
+218
| const TsFilePropertyValue& value = tsfile_property.second; | ||
| if (value.is_null) { | ||
| common::SerializationUtil::write_var_int(NO_STR_TO_READ, out); | ||
| } else { | ||
| common::SerializationUtil::write_var_int( | ||
| static_cast<int32_t>(value.value.size()), out); | ||
| if (!value.value.empty()) { | ||
| out.write_buf(value.value.data(), value.value.size()); | ||
| } | ||
| } |
Comment on lines
+126
to
+138
| int TsFileIOWriter::add_tsfile_property(const std::string& key, | ||
| const std::vector<uint8_t>& value) { | ||
| if (key.size() > static_cast<size_t>(std::numeric_limits<int32_t>::max()) || | ||
| value.size() > | ||
| static_cast<size_t>(std::numeric_limits<int32_t>::max())) { | ||
| return common::E_OUT_OF_RANGE; | ||
| } | ||
| if (file_ == nullptr || file_->get_fd() < 0) { | ||
| return common::E_FILE_WRITE_ERR; | ||
| } | ||
| tsfile_properties_[key] = TsFilePropertyValue(value); | ||
| return common::E_OK; | ||
| } |
Comment on lines
+263
to
+272
| ERRNO tsfile_writer_add_tsfile_property(TsFileWriter writer, const char* key, | ||
| uint32_t key_len, const uint8_t* value, | ||
| uint32_t value_len) { | ||
| if (writer == nullptr || key == nullptr || | ||
| (value == nullptr && value_len > 0)) { | ||
| return common::E_INVALID_ARG; | ||
| } | ||
| if (key_len > static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) { | ||
| return common::E_OUT_OF_RANGE; | ||
| } |
Comment on lines
+541
to
+543
| key = PyBytes_FromStringAndSize( | ||
| properties[i].key, properties[i].key_len | ||
| ).decode('utf-8') |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add file-level TsFile properties read/write support to the C++ and Python APIs.
Properties can be added or replaced while a writer is open, including after
flush(). Property values are stored as binary data and remain compatible with Java'sMap<String, byte[]>representation.Changes
C++
TsFilePropertyValueandTsFileProperties.TsFileWriterandTsFileTableWriter.TsFileReader::get_tsfile_properties().flush().encryptLevel,encryptType, andencryptKeyproperties override custom properties with the same names.TsFileMetawith value-based ownership.C wrapper
is_nullfield.tsfile_free_tsfile_properties()for releasing reader results.Python
add_tsfile_property(key: str, value: bytes)to:TsFileWriterTsFileTableWriterTsFileReader.get_tsfile_properties().dict[str, bytes | None].bytesvalues only; strings and other bytes-like objects are not converted implicitly.TsFileTreeWriterunchanged.Compatibility
The existing TsFile footer format is unchanged:
-1No property type tag is added. Applications are responsible for using an explicit, portable byte encoding for integers, floating-point values, and structures.
This change does not support modifying properties in an already closed TsFile.
Tests
Added coverage for:
flush()bytes | Nonereader resultsValidation completed:
./mvnw spotless:check -P with-cpp./mvnw -P with-cpp clean verify./mvnw -P with-python clean verify