Skip to content

Fix #16: add strict mode to reject unknown ViewDefinition select directives#17

Open
piotrszul wants to merge 1 commit into
masterfrom
fix/16_strict-unknown-directives
Open

Fix #16: add strict mode to reject unknown ViewDefinition select directives#17
piotrszul wants to merge 1 commit into
masterfrom
fix/16_strict-unknown-directives

Conversation

@piotrszul

Copy link
Copy Markdown
Collaborator

Fixes #16.

Problem

Unknown directives on a select element (e.g. repeat, which flatquack doesn't implement) were silently ignored. validateElement only inspected keys it recognized, so an element with an unknown key plus a column array passed as an ordinary column block, and parseNode dropped the directive entirely — producing a valid-but-wrong query (one all-NULL row per resource) instead of failing.

Fix

Add a strict option, default false (current lenient behavior unchanged), threaded parseVd → validateVd, plus a --strict CLI flag.

When strict is on, validateElement rejects any key on a select element that isn't a recognized directive (column/select/forEach/forEachOrNull/unionAll), naming the offender:

unsupported select directive: 'repeat'

The root ViewDefinition keys (resource/name/where/…) are unaffected — the strict check is applied only to select elements (the root validateElement(vd) call is not flagged as a select element).

Verification

End-to-end via the CLI on the issue's repeat view:

  • default: generates SQL, exit 0 (lenient, unchanged)
  • --strict: error: unsupported select directive: 'repeat'

Tests (tests/view.test.js)

  • strict mode rejects an unknown directive (repeat)
  • non-strict (default) ignores it
  • strict accepts a view using only known directives

All 194 tests pass. tests/spec-tests/ (reference fixtures) untouched.

Implementing repeat itself remains a separate, larger enhancement; this PR only makes the unsupported subset fail loudly when callers opt in.

Unknown directives on a ViewDefinition `select` element (e.g. `repeat`,
which flatquack does not implement) were silently ignored: validateElement
only inspected the keys it knew, so an unrecognized key on an element that
also had a `column` array passed as an ordinary column block, and parseNode
dropped the directive — compiling a valid-but-wrong query (one all-NULL row
per resource) instead of failing.

Add a `strict` option (default false, preserving current lenient behavior)
threaded through parseVd -> validateVd and a --strict CLI flag. When strict,
validateElement rejects any key on a select element that is not a recognized
directive (column/select/forEach/forEachOrNull/unionAll), naming the
offending key: "unsupported select directive: 'repeat'". The root
ViewDefinition keys (resource/name/where/...) are unaffected — the check
applies only to select elements.

Implementing `repeat` itself remains a separate, larger task; this makes
the unsupported subset fail loudly when callers opt in.

Tests (tests/view.test.js): strict rejects `repeat`; non-strict (default)
ignores it; strict accepts a view using only known directives. All 194
tests pass; tests/spec-tests/ untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@piotrszul piotrszul force-pushed the fix/16_strict-unknown-directives branch from a24bd02 to 709479b Compare June 9, 2026 19:17
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.

Unknown ViewDefinition select directives (e.g. repeat) are silently ignored — add a strict mode

1 participant