source-{various}: harden conditional reduction annotations for hard deletes#4362
Open
Alex-Bair wants to merge 1 commit into
Open
source-{various}: harden conditional reduction annotations for hard deletes#4362Alex-Bair wants to merge 1 commit into
Alex-Bair wants to merge 1 commit into
Conversation
The current deletion reduction annotation of:
```yaml
if:
properties:
_meta:
properties:
op: { const: "d" }
then:
reduce: { delete: true, strategy: merge }
else:
reduce: { strategy: merge }
```
also matches when either `_meta` or `op` are not present. Meaning, if
either of those are absent, the `if` conditional evaluates to `true` and
the `delete: true, strategy: merge` in the `then` branch is applied.
That's not what we want; we only want that deletion reduction to be
applied when when both fields are present and `op` is `"d"`.
This commit hardens the `if` check. By requiring `_meta` and `op` to be
present, the `if` conditional only evaluates to `true` when both are
present in the document _and_ `op` is `"d"`.
The multitude of snapshot updates are expected due to the additional
`required`s added to the discovered schemas.
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.
Description:
The current conditional reduction annotation of:
also matches when either
_metaoropare not present. Meaning, if either of those are absent, theifconditional evaluates totrueand thedelete: true, strategy: mergein thethenbranch is applied. That's not what we want; we only want that deletion reduction to be applied when both fields are present andopis"d".This PR hardens the
ifcheck. By requiring_metaandopto be present, theifconditional only evaluates totruewhen both are present in the document andopis"d":The multitude of snapshot updates are expected due to the additional
requireds added to the if/then/else block in discovered schemas.Notes for reviewers:
For reference on the if/then/else behavior, check out the official JSON schema docs. This callout on that page is relevant:
I'm not aware of any reported bugs due to the laxer matching behavior, likely because all documents emitted by these connectors have a
_metaand_meta/opfield present. I'd still like to close the gaps with these conditional reduction annotations so they're better references for future connectors or derivations that need to signal hard deletes via the same conditional reduction annotation.