fix(schema): require groupBy when having is present - #58
Merged
Merged
Conversation
Add dependentRequired { having: [groupBy] } to the root object and
minItems: 1 on groupBy, so having without grouping (or with an empty
groupBy) fails validation. Bump version to 0.1.0-preview.1.0.0 and
update CHANGELOG and README.
Closes #40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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.
Summary
Closes #40.
havingfilters aggregation groups, so it only makes sense alongsidegroupBy. The schema used to accepthavingon its own without complaint."dependentRequired": { "having": ["groupBy"] }to the root object. This draft 2020-12 keyword does the same job as theif/thenproposed in the issue, in one line.minItems: 1togroupBy, so"groupBy": []can't be used to get around the rule.CHANGELOG.mdentry under[Unreleased]listing both changes as breaking, and updatedREADME.md.Verification
All 21 samples still validate. Scenario checks against
samples/08_having.json:having+groupByhavingwithoutgroupBy'groupBy' is a dependency of 'having'having+ emptygroupBy[] is too shortRelease
This PR doesn't bump the version. When releasing, rename
[Unreleased]inCHANGELOG.mdto the new version and updateversion/$idin the schema. These changes tighten constraints, so the release needs a major bump.🤖 Generated with Claude Code