Add strict mode to vec merge, minor improvements and bug fixes - #60
Merged
Merged
Conversation
m-mohr
requested review from
ivorbosloper
and
a balanced review from Copilot
September 28, 2026 11:20
m-mohr
added this pull request to stack #57
September 28, 2026 11:21
ivorbosloper
approved these changes
Sep 28, 2026
ivorbosloper
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, but co-pilot has more remarks
Contributor
Author
|
Solved the remarks. |
m-mohr
removed this pull request from stack #57
September 28, 2026 11:39
m-mohr
added a commit
that referenced
this pull request
Sep 28, 2026
* Merge: hydrate only what the parts disagree on (review #1) * Merge: keep rows as they are with DuckDB, like in memory (review #2) * Report constants that don't fit their schema type (review #3) * Merge: reject two versions of one schema in a collection (review #4) * DuckDB: key ids by collection only with several collections (review #5) * Merge: apply excludes to the loaded data in memory (review #6) * DuckDB: quote required property names with _sql_name (review #7) * Share the CRS comparison of merge and DuckDB (review #8) * Merge: warn when includes drop a required collection-only property (review #9) * Merge: accept a collection that can't be determined (review #10) * Add strict mode to vec merge, minor improvements and bug fixes (#60) Co-authored-by: Matthias Mohr <m.mohr@moregeo.it>
m-mohr
added a commit
that referenced
this pull request
Sep 28, 2026
* Fix merge gaps, merge with DuckDB in vec merge * Merge: hydrate only what the parts disagree on (review #1) * Merge: keep rows as they are with DuckDB, like in memory (review #2) * Report constants that don't fit their schema type (review #3) * Merge: reject two versions of one schema in a collection (review #4) * DuckDB: key ids by collection only with several collections (review #5) * Merge: apply excludes to the loaded data in memory (review #6) * DuckDB: quote required property names with _sql_name (review #7) * Share the CRS comparison of merge and DuckDB (review #8) * Merge: warn when includes drop a required collection-only property (review #9) * Merge: accept a collection that can't be determined (review #10) * Add strict mode to vec merge, minor improvements and bug fixes (#60) Co-authored-by: Matthias Mohr <m.mohr@moregeo.it> Co-authored-by: Ivor <ivorbosloper@gmail.com>
Merged
m-mohr
added a commit
that referenced
this pull request
Sep 28, 2026
* merge_parquet: hydrate the constants the parts disagree on * Fix merge gaps, merge with DuckDB in vec merge * Merge: hydrate only what the parts disagree on (review #1) * Merge: keep rows as they are with DuckDB, like in memory (review #2) * Report constants that don't fit their schema type (review #3) * Merge: reject two versions of one schema in a collection (review #4) * DuckDB: key ids by collection only with several collections (review #5) * Merge: apply excludes to the loaded data in memory (review #6) * DuckDB: quote required property names with _sql_name (review #7) * Share the CRS comparison of merge and DuckDB (review #8) * Merge: warn when includes drop a required collection-only property (review #9) * Merge: accept a collection that can't be determined (review #10) * Add strict mode to vec merge, minor improvements and bug fixes (#60) * Error early when vecorel or extension versions diverge in a merge Co-authored-by: Matthias Mohr <m.mohr@moregeo.it> Co-authored-by: Ivor <ivorbosloper@gmail.com>
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.
Stacked on #58 and #56 and #55.
Adds a strict mode to
vec mergeon top of the review fixes in #58.vec mergeis strict by default: problems that make the merged dataset invalid are errors.--no-strictwarns and writes the dataset anyway. The README lists what each mode does per problem.merge_parquetgets an explicitstrict(defaultTrue).-i/-e, differing required collection-only properties.--no-strictthey are left empty instead of failing later in the writer.-i/-ealso catch required properties that only a custom schema requires.--no-strict, required columns with nulls are written as nullable instead of failing.Extract from the Readme:
By default,
vec mergeis strict: problems that make the merged dataset invalid are errorsand no dataset is written.
With
--no-strict,vec mergeis fail-safe: these problems are reported as warnings andthe dataset is written anyway. Check it with
vec validateafterwards.--no-strict-ior-eremoves a required propertyThe strict mode only checks what a merge can break or check with little effort.
It doesn't validate the values against the schemas, e.g. patterns or value ranges,
so a merged dataset is only valid if the values of the source datasets are valid.