You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "ChaomingZhangCN (via GitHub)" <gi...@apache.org> on 2026/10/16 03:20:35 UTC
[PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
ChaomingZhangCN opened a new pull request, #438:
URL: https://github.com/apache/paimon-cpp/pull/438
### Purpose
Linked issue: None.
Add dedicated Vortex storage for a single top-level VECTOR column in append-only tables, building on the Vortex format support in #376. With `vector.file.format=vortex`, ordinary columns are written to Parquet main files and the VECTOR column is written to `.vector.vortex` files. Both files cover the same row range and are read together through the existing data-evolution reader.
The dedicated mode requires:
```text
file.format=parquet
vector.file.format=vortex
bucket=-1
row-tracking.enabled=true
data-evolution.enabled=true
```
- Support full-row INSERTs with exactly one top-level VECTOR and at least one ordinary non-partition column.
- Roll main/VECTOR files together when either size target or the row-count target is reached; preserve matching row ranges and clean up both files on write/close failure.
- Add `vector.target-file-size`, defaulting to `target-file-size`.
- Reject unsupported dedicated-mode combinations: primary-key tables, custom/partial write schemas, multiple or nested VECTOR columns, and BLOB columns. Independent VECTOR file rolling is outside this implementation.
Separately, allow VECTOR columns in ordinary `file.format=vortex` tables. Validate visible VECTOR elements using ancestor validity before normalizing hidden NULL slots for Vortex import, so NULL STRUCT/LIST ancestors do not cause valid writes to fail. Visible NULL elements are still rejected for non-nullable VECTOR elements.
This is a scoped implementation of dedicated VECTOR storage; it does not claim complete feature parity with Paimon Java.
### Tests
Java interoperability is tested using checked-in files actually written and read back by Paimon Java `2.2-SNAPSHOT` at commit `7d6d146bbaf33a349dc83c3caf71334bd551e3d5` (Vortex JNI `0.73.0`). The fixture contains two append commits, six rows including two NULL vectors, and two Parquet/Vortex file pairs. Its generator, runtime dependency POM, provenance, and reproduction commands are included.
`DataEvolutionTableTest.TestReadJavaDedicatedVectorStore` reads the fixture without rewriting it and verifies full rows, ordinary-column and VECTOR-only projections, row ranges crossing commits, and paired file metadata (`firstRowId`, row counts, and `writeCols`). `VortexFileFormatTest.WriteThenReadVectorsWithNullAncestors` covers NULL STRUCT/LIST parents, batch boundaries, multiple read batch sizes, and rejection of the same invalid elements when the ancestors become visible. The NULL-ancestor regression fails before the fix and passes afterward.
Local validation on macOS aarch64, C++17 Debug, with Vortex enabled:
| Validation | Result |
| --- | --- |
| Core feature regression: rolling VECTOR writer, schema validation, options, append writer, row tracking, paths, BLOB rolling, and data evolution utilities | 118 passed at feature commit `dff3266` |
| Complete `paimon-data-evolution-table-test` after the fix | 294 passed, 12 skipped by format/parameter guards |
| Complete `paimon-vortex-format-test` after the fix | 26 passed |
| `paimon-write-and-read-inte-test --gtest_filter='Vortex*'` after the fix | 4 passed |
| Java fixture generation and Java readback assertions | Passed |
| Applicable changed-file pre-commit hooks and `git diff --check` | Passed |
Whole-file clang-format was skipped to preserve unrelated existing formatting; new C++ files and modified lines were checked with clang-format `20.1.8`. The existing local macOS build uses temporary dependency/link workarounds and still emits duplicate-library and deployment-target linker warnings. Linux CI, the repository-wide `unittest` target, sanitizers, and C++-written data read by Java have not been run.
### API and Format
- Add exported option constants `Options::VECTOR_FILE_FORMAT` and `Options::VECTOR_TARGET_FILE_SIZE` in `include/paimon/defs.h`.
- Reuse existing `.vector.vortex` naming and data-evolution metadata (`writeCols`, `firstRowId`, and row counts); no new manifest version or protocol is introduced.
- Dedicated storage is selected explicitly by `vector.file.format=vortex`; schemas and write contexts are validated before using this writer.
### Documentation
- Update `docs/source/user_guide/data_types.rst` with Vortex VECTOR support, dedicated-mode prerequisites, limitations, and paired rolling behavior.
- Add fixture provenance and reproducible Java generation instructions in `test/test_data/vortex/dedicated_vector_java_compat.db/dedicated_vector_java_compat/README.md`.
### Generative AI tooling
Generated-by: OpenAI Codex (codex-cli 0.162.0-alpha.17.2).
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on code in PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#discussion_r4323974658
##########
test/inte/blob_table_inte_test.cpp:
##########
@@ -1894,12 +1894,37 @@ TEST_P(BlobTableInteTest, TestDataEvolutionBlobPartialUpdateMultipleLayers) {
const std::vector<std::string> expected_rows = {
R"([[1, "a", "update1_0"]])", R"([[2, "b", "blob_1"]])", R"([[3, "c", "blob_2"]])",
R"([[4, "d", "update2_3"]])"};
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Plan> unpruned_plan, ScanTable(table_path));
+ ASSERT_EQ(unpruned_plan->Splits().size(), 1);
+ auto unpruned_split = std::dynamic_pointer_cast<DataSplitImpl>(unpruned_plan->Splits()[0]);
+ ASSERT_NE(unpruned_split, nullptr);
+ ASSERT_EQ(unpruned_split->DataFiles().size(), 4);
for (int32_t i = 0; i < static_cast<int32_t>(expected_rows.size()); i++) {
+ SCOPED_TRACE(i);
auto expected_single = std::dynamic_pointer_cast<arrow::StructArray>(
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields), expected_rows[i])
.ValueOrDie());
ASSERT_OK(ScanAndRead(table_path, schema->field_names(), expected_single,
/*predicate=*/nullptr, /*row_ranges=*/{Range(i, i)}));
+
+ // Range planning can narrow a read after scanning, leaving unselected BLOB files
+ // in the split. Their sequence layers must still align with the selected rows.
+ auto indexed_split =
+ std::make_shared<IndexedSplitImpl>(unpruned_split, std::vector<Range>{Range(i, i)});
+ ReadContextBuilder read_builder(table_path);
+ read_builder.SetReadFieldNames(schema->field_names()).EnableLateMaterializing(false);
+ ASSERT_OK_AND_ASSIGN(std::unique_ptr<ReadContext> read_context, read_builder.Finish());
Review Comment:
Updated the test to reuse ReadTable, preserving the regression coverage. Fixed in 491279e.
##########
src/paimon/core/io/rolling_vector_file_writer.cpp:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "paimon/core/io/rolling_vector_file_writer.h"
+
+#include "arrow/array/array_nested.h"
+#include "paimon/common/utils/arrow/status_utils.h"
+#include "paimon/common/utils/checked_cast.h"
+
+namespace paimon {
+
+RollingVectorFileWriter::RollingVectorFileWriter(
+ int64_t target_file_size, int64_t target_file_row_num, int64_t vector_target_file_size,
+ const std::shared_ptr<WriterFactory>& main_factory,
+ const std::shared_ptr<WriterFactory>& vector_factory,
+ const std::shared_ptr<arrow::Schema>& write_schema, int32_t vector_index)
+ : RollingFileWriter(target_file_size, target_file_row_num, main_factory),
+ vector_target_file_size_(vector_target_file_size),
+ vector_factory_(vector_factory),
+ write_schema_(write_schema),
+ vector_index_(vector_index) {}
+
+Status RollingVectorFileWriter::Write(::ArrowArray* record) {
+ if (closed_ || aborted_) {
+ return Status::Invalid("Writer has already closed or aborted!");
+ }
+ ScopeGuard guard([this]() { Abort(); });
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(
+ std::shared_ptr<arrow::Array> array,
+ arrow::ImportArray(record, arrow::struct_(write_schema_->fields())));
+ if (!array || array->type_id() != arrow::Type::STRUCT || array->null_count() != 0) {
+ return Status::Invalid("Dedicated VECTOR storage expects non-null struct rows");
+ }
+ if (array->length() == 0) {
+ guard.Release();
+ return Status::OK();
+ }
+ if (!current_writer_) {
+ PAIMON_RETURN_NOT_OK(OpenCurrentWriter());
+ PAIMON_ASSIGN_OR_RAISE(vector_writer_, vector_factory_->CreateWriter());
+ }
+ auto rows = checked_pointer_cast<arrow::StructArray>(array);
+ arrow::ArrayVector main_arrays;
+ arrow::FieldVector main_fields;
+ for (int32_t i = 0; i < write_schema_->num_fields(); ++i) {
+ if (i != vector_index_) {
+ main_arrays.push_back(rows->field(i));
+ main_fields.push_back(write_schema_->field(i));
+ }
+ }
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::StructArray> main_array,
+ arrow::StructArray::Make(main_arrays, main_fields));
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(
+ std::shared_ptr<arrow::StructArray> vector_array,
+ arrow::StructArray::Make({rows->field(vector_index_)},
+ {write_schema_->field(vector_index_)}));
+ ::ArrowArray main_batch = {};
+ ::ArrowArray vector_batch = {};
+ ScopeGuard arrays_guard([&]() {
+ ArrowArrayRelease(&main_batch);
+ ArrowArrayRelease(&vector_batch);
+ });
+ PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportArray(*main_array, &main_batch));
+ PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportArray(*vector_array, &vector_batch));
+ PAIMON_RETURN_NOT_OK(current_writer_->Write(&main_batch));
+ PAIMON_RETURN_NOT_OK(vector_writer_->Write(&vector_batch));
+ record_count_ += rows->length();
+ current_file_record_count_ += rows->length();
+
+ const bool suggested_check = SuggestCheck();
+ PAIMON_ASSIGN_OR_RAISE(bool main_full,
+ current_writer_->ReachTargetSize(suggested_check, target_file_size_));
+ PAIMON_ASSIGN_OR_RAISE(bool vector_full, vector_writer_->ReachTargetSize(
+ suggested_check, vector_target_file_size_));
+ if (main_full || vector_full || current_file_record_count_ >= target_file_row_num_) {
+ PAIMON_RETURN_NOT_OK(CloseCurrentPair());
Review Comment:
Updated the writer to roll VECTOR files independently. Rolling the main file also closes the active VECTOR file. Aligned Vortex's size estimate with Java and added regression tests for file ordering, row ID ranges, and failure cleanup. Fixed in 491279e.
##########
src/paimon/core/operation/data_evolution_split_read.cpp:
##########
@@ -674,7 +676,200 @@ DataEvolutionSplitRead::SplitFieldBunches(
return fields_files;
}
-Result<std::unique_ptr<DataEvolutionFileReader>> DataEvolutionSplitRead::CreateUnionReader(
+Result<std::vector<DataEvolutionSplitRead::VectorReadRange>>
+DataEvolutionSplitRead::PlanVectorReadRanges(
+ const std::vector<std::shared_ptr<DataFileMeta>>& files,
+ const std::vector<int32_t>& read_field_ids,
+ const std::function<Result<std::vector<int32_t>>(const std::shared_ptr<DataFileMeta>&)>&
+ file_to_field_ids) {
+ struct Candidate {
+ std::shared_ptr<DataFileMeta> file;
+ std::vector<int32_t> field_ids;
+ };
+ std::vector<Candidate> candidates;
+ std::vector<std::shared_ptr<DataFileMeta>> other_files;
+ std::map<int64_t, std::vector<size_t>> boundaries;
+ std::unordered_set<int32_t> projected_ids(read_field_ids.begin(), read_field_ids.end());
+ std::optional<Range> logical_range;
+ int64_t first_row_id = std::numeric_limits<int64_t>::max();
+ int64_t end_row_id = std::numeric_limits<int64_t>::min();
+ bool vector_only = std::all_of(files.begin(), files.end(), [](const auto& file) {
+ return VectorStoreUtils::IsVectorStoreFile(file->file_name);
+ });
+ for (const auto& file : files) {
+ PAIMON_ASSIGN_OR_RAISE(int64_t first, file->NonNullFirstRowId());
+ if (first < 0 || file->row_count <= 0 ||
+ first > std::numeric_limits<int64_t>::max() - file->row_count) {
+ return Status::Invalid("Invalid row id range in VECTOR merge group");
+ }
+ int64_t end = first + file->row_count;
+ first_row_id = std::min(first_row_id, first);
+ end_row_id = std::max(end_row_id, end);
+ if (!VectorStoreUtils::IsVectorStoreFile(file->file_name)) {
+ other_files.push_back(file);
+ if (DataEvolutionUtils::IsNormalFile(file->file_name)) {
+ logical_range = Range(first, end - 1);
+ }
+ continue;
+ }
+ PAIMON_ASSIGN_OR_RAISE(std::vector<int32_t> field_ids, file_to_field_ids(file));
+ field_ids.erase(std::remove_if(field_ids.begin(), field_ids.end(),
+ [&](int32_t id) { return projected_ids.count(id) == 0; }),
+ field_ids.end());
+ if (!field_ids.empty() || vector_only) {
+ size_t index = candidates.size();
+ candidates.push_back(Candidate{file, std::move(field_ids)});
+ boundaries[first].push_back(index);
+ boundaries[end].push_back(index);
+ }
+ }
+ if (files.empty()) {
+ return std::vector<VectorReadRange>();
+ }
+ if (!logical_range) {
+ logical_range = Range(first_row_id, end_row_id - 1);
+ }
+ boundaries[logical_range->from];
+ boundaries[logical_range->to + 1];
+
+ auto newest_first = [&](size_t left, size_t right) {
+ const auto& lhs = candidates[left].file;
+ const auto& rhs = candidates[right].file;
+ if (lhs->max_sequence_number != rhs->max_sequence_number) {
+ return lhs->max_sequence_number > rhs->max_sequence_number;
+ }
+ if (lhs->file_name != rhs->file_name) {
+ return lhs->file_name < rhs->file_name;
+ }
+ return left < right;
+ };
+ std::set<size_t, decltype(newest_first)> active(newest_first);
+ std::vector<VectorReadRange> result;
+ int64_t start = boundaries.begin()->first;
+ for (const auto& [end, events] : boundaries) {
+ int64_t from = std::max(start, logical_range->from);
+ int64_t to = std::min(end - 1, logical_range->to);
+ if (from <= to) {
+ std::unordered_set<int32_t> assigned;
+ std::vector<std::shared_ptr<DataFileMeta>> providers = other_files;
+ for (size_t index : active) {
+ const auto& candidate = candidates[index];
+ bool provides_field = false;
+ for (int32_t id : candidate.field_ids) {
+ provides_field |= assigned.insert(id).second;
+ }
+ // An explicit NULL in a newer file overrides the older value as well.
+ if (provides_field) {
+ providers.push_back(candidate.file);
+ }
+ }
+ if (providers.empty() && vector_only && !active.empty()) {
+ providers.push_back(candidates[*active.begin()].file);
+ }
+ // Ordinary inline fields and dedicated fields share the same sequence precedence.
+ // BLOB files remain last and retain their own per-row fallback ordering.
+ std::stable_sort(
+ providers.begin(), providers.end(), [](const auto& lhs, const auto& rhs) {
+ bool lhs_blob = BlobUtils::IsBlobFile(lhs->file_name);
+ bool rhs_blob = BlobUtils::IsBlobFile(rhs->file_name);
+ if (lhs_blob != rhs_blob) {
+ return !lhs_blob;
+ }
+ return !lhs_blob && lhs->max_sequence_number > rhs->max_sequence_number;
+ });
+ if (!result.empty() && result.back().range.to + 1 == from &&
+ result.back().files == providers) {
+ result.back().range.to = to;
+ } else {
+ result.push_back(VectorReadRange{Range(from, to), std::move(providers)});
+ }
+ }
+ for (size_t index : events) {
+ if (active.erase(index) == 0) {
+ active.insert(index);
+ }
+ }
+ start = end;
+ }
+ return result;
Review Comment:
Added an explanation and a TODO for a sequential fast path to avoid reopening the main file across ranges. Fixed in 491279e.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated storage for append-only tables [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on code in PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#discussion_r4332192048
##########
src/paimon/core/operation/data_evolution_split_read.cpp:
##########
@@ -674,7 +676,203 @@ DataEvolutionSplitRead::SplitFieldBunches(
return fields_files;
}
Review Comment:
Could we also update `DataEvolutionFileStoreScan::EvolutionStats` for independently rolled VECTOR files? It currently excludes only BLOB files, sorts the remaining files by sequence number, and uses `entries[0].File()->row_count` as the entire group’s row count.
For example, a main file contains 10,000 rows with 1,000 NULLs in `tag`. A later full-range update of `embedding` produces independently rolled VECTOR files of 1,000 rows each. A newer VECTOR file sorts first, so `tag IS NOT NULL` is evaluated using the main file’s null count but the VECTOR file’s row count: `1000 < 1000` is false. This incorrectly prunes the entire group, losing 9,000 matching rows.
Java computes the logical group row count and excludes BLOB/VECTOR files from this statistics merge. Could we align this logic and add a regression test covering newer rolled VECTOR files, nullable ordinary columns, and predicate pruning?
##########
test/test_data/vortex/dedicated_vector_java_compat.db/dedicated_vector_java_compat/README.md:
##########
@@ -0,0 +1,58 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
Review Comment:
Please align the other READMEs by removing the license description.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#issuecomment-6258866315
> Thanks for adding dedicated VECTOR storage support and the Java interoperability fixture!
>
> As I understand it, this implementation leaves the following cases unsupported in dedicated-storage mode:
>
> * Multiple top-level VECTOR columns
> * Nested VECTOR columns
> * Partial-column writes or custom write schemas
> * BLOB columns alongside VECTOR columns
> * Independent rolling of main and VECTOR files
> * Main file formats other than Parquet, or VECTOR file formats other than Vortex
>
> Keeping the scope focused makes sense. Would you consider extending support for some of these cases in follow-up PRs? It would be helpful to understand the intended direction, particularly for multiple VECTOR columns, partial writes, and independent file rolling.
Yes, these are areas I’d like to extend in follow-up PRs.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on code in PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#discussion_r4323973419
##########
test/test_data/vortex/dedicated_vector_java_rolling.db/dedicated_vector_java_rolling/GenerateDedicatedVectorRollingFixture.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.CatalogContext;
+import org.apache.paimon.catalog.CatalogFactory;
+import org.apache.paimon.catalog.Identifier;
+import org.apache.paimon.data.BinaryString;
+import org.apache.paimon.data.BinaryVector;
+import org.apache.paimon.data.GenericRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.manifest.ManifestEntry;
+import org.apache.paimon.options.Options;
+import org.apache.paimon.schema.Schema;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.Table;
+import org.apache.paimon.table.sink.BatchTableCommit;
+import org.apache.paimon.table.sink.BatchTableWrite;
+import org.apache.paimon.table.sink.BatchWriteBuilder;
+import org.apache.paimon.table.source.ReadBuilder;
+import org.apache.paimon.types.DataTypes;
+import org.apache.paimon.utils.CloseableIterator;
+
+import java.util.Arrays;
+import java.util.BitSet;
+import java.util.Map;
+import java.util.TreeMap;
+
+/** Generates one Java main file with independently rolled dedicated VECTOR files. */
Review Comment:
Removed the Java generators and POMs from both fixtures. Kept the generated data and documented how to construct it in the READMEs. Fixed in 491279e.
##########
test/test_data/vortex/dedicated_vector_java_rolling.db/dedicated_vector_java_rolling/README.md:
##########
@@ -0,0 +1,128 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
Review Comment:
Simplified the READMEs to match the existing fixture style, covering the schema, options, data pattern, and commit/file layout. Fixed in 491279e.
##########
src/paimon/core/core_options.cpp:
##########
@@ -599,6 +601,15 @@ struct CoreOptions::Impl {
// Parse file.format - data file format, default "parquet"
PAIMON_RETURN_NOT_OK(parser.ParseObject<FileFormatFactory>(
Options::FILE_FORMAT, /*default_identifier=*/"parquet", &file_format));
+ if (parser.ContainsKey(Options::VECTOR_FILE_FORMAT)) {
+ PAIMON_RETURN_NOT_OK(parser.ParseObject<FileFormatFactory>(
+ Options::VECTOR_FILE_FORMAT, /*default_identifier=*/"vortex", &vector_file_format));
+ }
+ PAIMON_RETURN_NOT_OK(
+ parser.ParseMemorySize(Options::VECTOR_TARGET_FILE_SIZE, &vector_target_file_size));
+ if (vector_target_file_size && vector_target_file_size.value() <= 0) {
+ return Status::Invalid("vector.target-file-size must be positive");
+ }
Review Comment:
Added tests for the default and explicitly configured values of both VECTOR options in TestDefaultValue and TestFromMap. Fixed in 491279e.
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -1261,6 +1263,58 @@ Status SchemaValidation::ValidateMapStorageLayout(const TableSchema& schema,
return Status::OK();
}
+Status SchemaValidation::ValidateVectorStore(const TableSchema& schema,
+ const CoreOptions& options) {
+ const std::shared_ptr<FileFormat> vector_format = options.GetVectorFileFormat();
+ if (!vector_format) {
+ return Status::OK();
+ }
+ if (vector_format->Identifier() != "vortex") {
+ return Status::NotImplemented("Dedicated VECTOR storage currently only supports vortex");
Review Comment:
Removed the Vortex-only restriction and added parameterized tests for Parquet, Lance, and Vortex. Parquet and Vortex passed locally; Lance runtime tests were not run locally. Fixed in 491279e.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated storage for append-only tables [paimon-cpp]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on code in PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#discussion_r4334567218
##########
src/paimon/core/operation/data_evolution_split_read.cpp:
##########
@@ -674,7 +676,203 @@ DataEvolutionSplitRead::SplitFieldBunches(
return fields_files;
}
Review Comment:
Fixed in 9e562e8. EvolutionStats now computes the logical group row count and excludes BLOB/VECTOR files from statistics merging. Added regression coverage for newer rolled VECTOR files, nullable ordinary columns and scan pruning, plus dedicated-only and partial-range groups. Relevant tests pass.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#issuecomment-6245651613
Java rolls VECTOR files independently, so one main file can correspond to multiple VECTOR files with different row ranges—for example, a main file covering `[0,9999]` and two VECTOR files covering `[0,999]` and `[1000,9999]`.
However, `DataEvolutionSplitRead::MergeRangesAndSort` still treats VECTOR files as ordinary data files and requires their row ranges to be identical. This rejects valid Java-written tables with `Data files should be all row id ranges same.`, even when projecting only ordinary columns.
Could we add the corresponding VECTOR grouping/concatenation logic and an interoperability fixture with multiple VECTOR files per main file? The current fixture only covers one-to-one pairs with identical ranges, so it does not exercise this case.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#issuecomment-6258595500
> Java rolls VECTOR files independently, so one main file can correspond to multiple VECTOR files with different row ranges—for example, a main file covering `[0,9999]` and two VECTOR files covering `[0,999]` and `[1000,9999]`.
>
> However, `DataEvolutionSplitRead::MergeRangesAndSort` still treats VECTOR files as ordinary data files and requires their row ranges to be identical. This rejects valid Java-written tables with `Data files should be all row id ranges same.`, even when projecting only ordinary columns.
>
> Could we add the corresponding VECTOR grouping/concatenation logic and an interoperability fixture with multiple VECTOR files per main file? The current fixture only covers one-to-one pairs with identical ranges, so it does not exercise this case.
Thanks for catching this. Fixed in 5e9fb80: the reader now aligns row-id ranges to support independently rolled VECTOR files. Added a Java-generated fixture with 1 main file and 4 VECTOR files (3,001 rows), covering scalar/vector projections, NULLs, and row-range selection. Relevant tests pass.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on code in PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#discussion_r4312972867
##########
src/paimon/core/core_options.cpp:
##########
@@ -599,6 +601,15 @@ struct CoreOptions::Impl {
// Parse file.format - data file format, default "parquet"
PAIMON_RETURN_NOT_OK(parser.ParseObject<FileFormatFactory>(
Options::FILE_FORMAT, /*default_identifier=*/"parquet", &file_format));
+ if (parser.ContainsKey(Options::VECTOR_FILE_FORMAT)) {
+ PAIMON_RETURN_NOT_OK(parser.ParseObject<FileFormatFactory>(
+ Options::VECTOR_FILE_FORMAT, /*default_identifier=*/"vortex", &vector_file_format));
+ }
+ PAIMON_RETURN_NOT_OK(
+ parser.ParseMemorySize(Options::VECTOR_TARGET_FILE_SIZE, &vector_target_file_size));
+ if (vector_target_file_size && vector_target_file_size.value() <= 0) {
+ return Status::Invalid("vector.target-file-size must be positive");
+ }
Review Comment:
Please add tests for these two interfaces in `core_options_test.cpp`. May only need to add two lines for `TestDefaultValue` and `TestFromMap`.
##########
test/test_data/vortex/dedicated_vector_java_rolling.db/dedicated_vector_java_rolling/README.md:
##########
@@ -0,0 +1,128 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
Review Comment:
Please refer to the `README` of other test data and align the format.
##########
test/test_data/vortex/dedicated_vector_java_rolling.db/dedicated_vector_java_rolling/GenerateDedicatedVectorRollingFixture.java:
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.CatalogContext;
+import org.apache.paimon.catalog.CatalogFactory;
+import org.apache.paimon.catalog.Identifier;
+import org.apache.paimon.data.BinaryString;
+import org.apache.paimon.data.BinaryVector;
+import org.apache.paimon.data.GenericRow;
+import org.apache.paimon.data.InternalRow;
+import org.apache.paimon.io.DataFileMeta;
+import org.apache.paimon.manifest.ManifestEntry;
+import org.apache.paimon.options.Options;
+import org.apache.paimon.schema.Schema;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.Table;
+import org.apache.paimon.table.sink.BatchTableCommit;
+import org.apache.paimon.table.sink.BatchTableWrite;
+import org.apache.paimon.table.sink.BatchWriteBuilder;
+import org.apache.paimon.table.source.ReadBuilder;
+import org.apache.paimon.types.DataTypes;
+import org.apache.paimon.utils.CloseableIterator;
+
+import java.util.Arrays;
+import java.util.BitSet;
+import java.util.Map;
+import java.util.TreeMap;
+
+/** Generates one Java main file with independently rolled dedicated VECTOR files. */
Review Comment:
Please remove this Java file and the pom file. We want the data construction pattern to be understandable via the README alone.
##########
src/paimon/core/io/rolling_vector_file_writer.cpp:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "paimon/core/io/rolling_vector_file_writer.h"
+
+#include "arrow/array/array_nested.h"
+#include "paimon/common/utils/arrow/status_utils.h"
+#include "paimon/common/utils/checked_cast.h"
+
+namespace paimon {
+
+RollingVectorFileWriter::RollingVectorFileWriter(
+ int64_t target_file_size, int64_t target_file_row_num, int64_t vector_target_file_size,
+ const std::shared_ptr<WriterFactory>& main_factory,
+ const std::shared_ptr<WriterFactory>& vector_factory,
+ const std::shared_ptr<arrow::Schema>& write_schema, int32_t vector_index)
+ : RollingFileWriter(target_file_size, target_file_row_num, main_factory),
+ vector_target_file_size_(vector_target_file_size),
+ vector_factory_(vector_factory),
+ write_schema_(write_schema),
+ vector_index_(vector_index) {}
+
+Status RollingVectorFileWriter::Write(::ArrowArray* record) {
+ if (closed_ || aborted_) {
+ return Status::Invalid("Writer has already closed or aborted!");
+ }
+ ScopeGuard guard([this]() { Abort(); });
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(
+ std::shared_ptr<arrow::Array> array,
+ arrow::ImportArray(record, arrow::struct_(write_schema_->fields())));
+ if (!array || array->type_id() != arrow::Type::STRUCT || array->null_count() != 0) {
+ return Status::Invalid("Dedicated VECTOR storage expects non-null struct rows");
+ }
+ if (array->length() == 0) {
+ guard.Release();
+ return Status::OK();
+ }
+ if (!current_writer_) {
+ PAIMON_RETURN_NOT_OK(OpenCurrentWriter());
+ PAIMON_ASSIGN_OR_RAISE(vector_writer_, vector_factory_->CreateWriter());
+ }
+ auto rows = checked_pointer_cast<arrow::StructArray>(array);
+ arrow::ArrayVector main_arrays;
+ arrow::FieldVector main_fields;
+ for (int32_t i = 0; i < write_schema_->num_fields(); ++i) {
+ if (i != vector_index_) {
+ main_arrays.push_back(rows->field(i));
+ main_fields.push_back(write_schema_->field(i));
+ }
+ }
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::StructArray> main_array,
+ arrow::StructArray::Make(main_arrays, main_fields));
+ PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(
+ std::shared_ptr<arrow::StructArray> vector_array,
+ arrow::StructArray::Make({rows->field(vector_index_)},
+ {write_schema_->field(vector_index_)}));
+ ::ArrowArray main_batch = {};
+ ::ArrowArray vector_batch = {};
+ ScopeGuard arrays_guard([&]() {
+ ArrowArrayRelease(&main_batch);
+ ArrowArrayRelease(&vector_batch);
+ });
+ PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportArray(*main_array, &main_batch));
+ PAIMON_RETURN_NOT_OK_FROM_ARROW(arrow::ExportArray(*vector_array, &vector_batch));
+ PAIMON_RETURN_NOT_OK(current_writer_->Write(&main_batch));
+ PAIMON_RETURN_NOT_OK(vector_writer_->Write(&vector_batch));
+ record_count_ += rows->length();
+ current_file_record_count_ += rows->length();
+
+ const bool suggested_check = SuggestCheck();
+ PAIMON_ASSIGN_OR_RAISE(bool main_full,
+ current_writer_->ReachTargetSize(suggested_check, target_file_size_));
+ PAIMON_ASSIGN_OR_RAISE(bool vector_full, vector_writer_->ReachTargetSize(
+ suggested_check, vector_target_file_size_));
+ if (main_full || vector_full || current_file_record_count_ >= target_file_row_num_) {
+ PAIMON_RETURN_NOT_OK(CloseCurrentPair());
Review Comment:
This behavior isn’t consistent with Java. In Java, the VECTOR writer itself is a `RollingFileWriterImpl`: when the VECTOR size reaches its target, it only rolls the VECTOR files; the VECTOR writer is only closed when the main file rolls. However, the current C++ implementation closes both sides together whenever either side hits its size/line targets.
##########
test/inte/blob_table_inte_test.cpp:
##########
@@ -1894,12 +1894,37 @@ TEST_P(BlobTableInteTest, TestDataEvolutionBlobPartialUpdateMultipleLayers) {
const std::vector<std::string> expected_rows = {
R"([[1, "a", "update1_0"]])", R"([[2, "b", "blob_1"]])", R"([[3, "c", "blob_2"]])",
R"([[4, "d", "update2_3"]])"};
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<Plan> unpruned_plan, ScanTable(table_path));
+ ASSERT_EQ(unpruned_plan->Splits().size(), 1);
+ auto unpruned_split = std::dynamic_pointer_cast<DataSplitImpl>(unpruned_plan->Splits()[0]);
+ ASSERT_NE(unpruned_split, nullptr);
+ ASSERT_EQ(unpruned_split->DataFiles().size(), 4);
for (int32_t i = 0; i < static_cast<int32_t>(expected_rows.size()); i++) {
+ SCOPED_TRACE(i);
auto expected_single = std::dynamic_pointer_cast<arrow::StructArray>(
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields), expected_rows[i])
.ValueOrDie());
ASSERT_OK(ScanAndRead(table_path, schema->field_names(), expected_single,
/*predicate=*/nullptr, /*row_ranges=*/{Range(i, i)}));
+
+ // Range planning can narrow a read after scanning, leaving unselected BLOB files
+ // in the split. Their sequence layers must still align with the selected rows.
+ auto indexed_split =
+ std::make_shared<IndexedSplitImpl>(unpruned_split, std::vector<Range>{Range(i, i)});
+ ReadContextBuilder read_builder(table_path);
+ read_builder.SetReadFieldNames(schema->field_names()).EnableLateMaterializing(false);
+ ASSERT_OK_AND_ASSIGN(std::unique_ptr<ReadContext> read_context, read_builder.Finish());
Review Comment:
Can we just use the `ReadTable` function here?
##########
src/paimon/core/operation/data_evolution_split_read.cpp:
##########
@@ -674,7 +676,200 @@ DataEvolutionSplitRead::SplitFieldBunches(
return fields_files;
}
-Result<std::unique_ptr<DataEvolutionFileReader>> DataEvolutionSplitRead::CreateUnionReader(
+Result<std::vector<DataEvolutionSplitRead::VectorReadRange>>
+DataEvolutionSplitRead::PlanVectorReadRanges(
+ const std::vector<std::shared_ptr<DataFileMeta>>& files,
+ const std::vector<int32_t>& read_field_ids,
+ const std::function<Result<std::vector<int32_t>>(const std::shared_ptr<DataFileMeta>&)>&
+ file_to_field_ids) {
+ struct Candidate {
+ std::shared_ptr<DataFileMeta> file;
+ std::vector<int32_t> field_ids;
+ };
+ std::vector<Candidate> candidates;
+ std::vector<std::shared_ptr<DataFileMeta>> other_files;
+ std::map<int64_t, std::vector<size_t>> boundaries;
+ std::unordered_set<int32_t> projected_ids(read_field_ids.begin(), read_field_ids.end());
+ std::optional<Range> logical_range;
+ int64_t first_row_id = std::numeric_limits<int64_t>::max();
+ int64_t end_row_id = std::numeric_limits<int64_t>::min();
+ bool vector_only = std::all_of(files.begin(), files.end(), [](const auto& file) {
+ return VectorStoreUtils::IsVectorStoreFile(file->file_name);
+ });
+ for (const auto& file : files) {
+ PAIMON_ASSIGN_OR_RAISE(int64_t first, file->NonNullFirstRowId());
+ if (first < 0 || file->row_count <= 0 ||
+ first > std::numeric_limits<int64_t>::max() - file->row_count) {
+ return Status::Invalid("Invalid row id range in VECTOR merge group");
+ }
+ int64_t end = first + file->row_count;
+ first_row_id = std::min(first_row_id, first);
+ end_row_id = std::max(end_row_id, end);
+ if (!VectorStoreUtils::IsVectorStoreFile(file->file_name)) {
+ other_files.push_back(file);
+ if (DataEvolutionUtils::IsNormalFile(file->file_name)) {
+ logical_range = Range(first, end - 1);
+ }
+ continue;
+ }
+ PAIMON_ASSIGN_OR_RAISE(std::vector<int32_t> field_ids, file_to_field_ids(file));
+ field_ids.erase(std::remove_if(field_ids.begin(), field_ids.end(),
+ [&](int32_t id) { return projected_ids.count(id) == 0; }),
+ field_ids.end());
+ if (!field_ids.empty() || vector_only) {
+ size_t index = candidates.size();
+ candidates.push_back(Candidate{file, std::move(field_ids)});
+ boundaries[first].push_back(index);
+ boundaries[end].push_back(index);
+ }
+ }
+ if (files.empty()) {
+ return std::vector<VectorReadRange>();
+ }
+ if (!logical_range) {
+ logical_range = Range(first_row_id, end_row_id - 1);
+ }
+ boundaries[logical_range->from];
+ boundaries[logical_range->to + 1];
+
+ auto newest_first = [&](size_t left, size_t right) {
+ const auto& lhs = candidates[left].file;
+ const auto& rhs = candidates[right].file;
+ if (lhs->max_sequence_number != rhs->max_sequence_number) {
+ return lhs->max_sequence_number > rhs->max_sequence_number;
+ }
+ if (lhs->file_name != rhs->file_name) {
+ return lhs->file_name < rhs->file_name;
+ }
+ return left < right;
+ };
+ std::set<size_t, decltype(newest_first)> active(newest_first);
+ std::vector<VectorReadRange> result;
+ int64_t start = boundaries.begin()->first;
+ for (const auto& [end, events] : boundaries) {
+ int64_t from = std::max(start, logical_range->from);
+ int64_t to = std::min(end - 1, logical_range->to);
+ if (from <= to) {
+ std::unordered_set<int32_t> assigned;
+ std::vector<std::shared_ptr<DataFileMeta>> providers = other_files;
+ for (size_t index : active) {
+ const auto& candidate = candidates[index];
+ bool provides_field = false;
+ for (int32_t id : candidate.field_ids) {
+ provides_field |= assigned.insert(id).second;
+ }
+ // An explicit NULL in a newer file overrides the older value as well.
+ if (provides_field) {
+ providers.push_back(candidate.file);
+ }
+ }
+ if (providers.empty() && vector_only && !active.empty()) {
+ providers.push_back(candidates[*active.begin()].file);
+ }
+ // Ordinary inline fields and dedicated fields share the same sequence precedence.
+ // BLOB files remain last and retain their own per-row fallback ordering.
+ std::stable_sort(
+ providers.begin(), providers.end(), [](const auto& lhs, const auto& rhs) {
+ bool lhs_blob = BlobUtils::IsBlobFile(lhs->file_name);
+ bool rhs_blob = BlobUtils::IsBlobFile(rhs->file_name);
+ if (lhs_blob != rhs_blob) {
+ return !lhs_blob;
+ }
+ return !lhs_blob && lhs->max_sequence_number > rhs->max_sequence_number;
+ });
+ if (!result.empty() && result.back().range.to + 1 == from &&
+ result.back().files == providers) {
+ result.back().range.to = to;
+ } else {
+ result.push_back(VectorReadRange{Range(from, to), std::move(providers)});
+ }
+ }
+ for (size_t index : events) {
+ if (active.erase(index) == 0) {
+ active.insert(index);
+ }
+ }
+ start = end;
+ }
+ return result;
Review Comment:
For a single main file covered by multiple independently rolled VECTOR files, this implementation appears to create a separate main-file reader for each planned range when projecting both regular and vector columns. Java keeps the sequential read path for contiguous, non-overlapping VECTOR files, avoiding these repeated opens.
Could we at least add a code comment documenting this difference and a TODO to preserve the sequential path for this layout? That would make the current trade-off explicit and help track the follow-up optimization.
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -1261,6 +1263,58 @@ Status SchemaValidation::ValidateMapStorageLayout(const TableSchema& schema,
return Status::OK();
}
+Status SchemaValidation::ValidateVectorStore(const TableSchema& schema,
+ const CoreOptions& options) {
+ const std::shared_ptr<FileFormat> vector_format = options.GetVectorFileFormat();
+ if (!vector_format) {
+ return Status::OK();
+ }
+ if (vector_format->Identifier() != "vortex") {
+ return Status::NotImplemented("Dedicated VECTOR storage currently only supports vortex");
Review Comment:
It seems Java’s independently configured dedicated VECTOR storage isn’t limited to Vortex—Lance and even Parquet support it as well. Why do we want to enforce a specific format here?
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated storage for append-only tables [paimon-cpp]
Posted by "ChaomingZhangCN (via GitHub)" <gi...@apache.org>.
ChaomingZhangCN commented on code in PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#discussion_r4334564705
##########
test/test_data/vortex/dedicated_vector_java_compat.db/dedicated_vector_java_compat/README.md:
##########
@@ -0,0 +1,58 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
Review Comment:
Removed the license preambles from both fixture READMEs to match the existing test-data format. Fixed in 9e562e8.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated storage for append-only tables [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 merged PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(vector): add dedicated Vortex storage for append-only tables [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on PR #438:
URL: https://github.com/apache/paimon-cpp/pull/438#issuecomment-6245677628
Thanks for adding dedicated VECTOR storage support and the Java interoperability fixture!
As I understand it, this implementation leaves the following cases unsupported in dedicated-storage mode:
- Multiple top-level VECTOR columns
- Nested VECTOR columns
- Partial-column writes or custom write schemas
- BLOB columns alongside VECTOR columns
- Independent rolling of main and VECTOR files
- Main file formats other than Parquet, or VECTOR file formats other than Vortex
Keeping the scope focused makes sense. Would you consider extending support for some of these cases in follow-up PRs? It would be helpful to understand the intended direction, particularly for multiple VECTOR columns, partial writes, and independent file rolling.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org