feat: add rewrite manifests#774
Conversation
18bade1 to
7de6949
Compare
5c1ee1f to
dd9c7e6
Compare
| auto new_writer = [this, &schema](const RewriteWriter& rewrite_writer) | ||
| -> Result<std::unique_ptr<ManifestWriter>> { | ||
| std::optional<int64_t> first_row_id = std::nullopt; | ||
| if (base().format_version >= 3 && rewrite_writer.content == ManifestContent::kData) { |
There was a problem hiding this comment.
If the table was upgraded from v2, existing live files may still have null per-file row IDs. Then ManifestEntryAdapterV3::GetFirstRowId falls back to the writer-level value, so those files get 0, and the manifest list skips assignment because the manifest now has a non-null first_row_id. Java leaves the rewrite writer’s manifest firstRowId null via newManifestWriter, allowing manifest-list assignment when needed after upgrade.
There was a problem hiding this comment.
Good catch. I removed the non-null rewrite writer first_row_id logic and added a v2 -> v3 regression test.
| const bool has_direct_replacements = !deleted_manifests_.empty() || | ||
| !added_manifests_.empty() || | ||
| !rewritten_added_manifests_.empty(); | ||
| if (!cluster_by_func_ && !predicate_ && has_direct_replacements) { |
There was a problem hiding this comment.
Java does not rewrite at all when clusterByFunc == null, while this rewrites when predicate_ is set. Is it intentional?
There was a problem hiding this comment.
Not intentional. I changed RequiresRewrite to match Java.
dd9c7e6 to
3736bc1
Compare
No description provided.