Skip to content

Add strict mode to vec merge, minor improvements and bug fixes - #60

Merged
m-mohr merged 2 commits into
merge-fixes-reviewfrom
merge-strict
Sep 28, 2026
Merged

m-mohr merged 2 commits into
merge-fixes-reviewfrom
merge-strict

Conversation

@m-mohr

@m-mohr m-mohr commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #58 and #56 and #55.

Adds a strict mode to vec merge on top of the review fixes in #58.

  • Breaking: vec merge is strict by default: problems that make the merged dataset invalid are errors. --no-strict warns and writes the dataset anyway. The README lists what each mode does per problem. merge_parquet gets an explicit strict (default True).
  • Both engines report the same problems: missing required values (per collection), repeating ids, empty geometries, undeterminable collections, required properties removed by -i/-e, differing required collection-only properties.
  • Constants that don't fit their schema type are reported with property and file in both engines; with --no-strict they are left empty instead of failing later in the writer.
  • -i/-e also catch required properties that only a custom schema requires.
  • With --no-strict, required columns with nulls are written as nullable instead of failing.
  • Missing required values are reported per property with counts.

Extract from the Readme:

By default, vec merge is strict: problems that make the merged dataset invalid are errors
and no dataset is written.
With --no-strict, vec merge is fail-safe: these problems are reported as warnings and
the dataset is written anyway. Check it with vec validate afterwards.

Problem Strict (default) --no-strict
A required property has no value for some features Error Warning, the property is written as nullable
An id repeats within a collection Error Warning
The collection of a feature can't be determined Error Warning, the collection is left empty
-i or -e removes a required property Error Warning
A required collection-only property differs between the datasets Error Warning, the property is removed
An optional collection-only property differs between the datasets Warning, the property is removed Warning, the property is removed
A feature has an empty or missing geometry Error Warning, the feature is kept
A collection-level value doesn't fit the data type of its schema Error Warning, the value is left empty
A collection implements multiple versions of a schema, e.g. of an extension Error Error
The schemas of the datasets conflict Error Error

The 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.

@m-mohr
m-mohr requested review from ivorbosloper and a balanced review from Copilot September 28, 2026 11:20
@m-mohr
m-mohr added this pull request to stack #57 September 28, 2026 11:21

This comment was marked as resolved.

@ivorbosloper ivorbosloper left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but co-pilot has more remarks

@m-mohr

m-mohr commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Solved the remarks.

@m-mohr
m-mohr removed this pull request from stack #57 September 28, 2026 11:39
@m-mohr
m-mohr merged commit daaa30f into merge-fixes-review Sep 28, 2026
7 checks passed
@m-mohr
m-mohr deleted the merge-strict branch 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>
@m-mohr m-mohr mentioned this pull request Sep 28, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants