Update Aggregate Projection Restrictions. by kasei · Pull Request #380 · w3c/sparql-query

kasei · 2026-05-22T16:10:23Z

Since there hasn't been any discussion on #377, I'm moving the proposed changes to this PR. Copying from that issue:

The big changes here are:

explicitly includes any query level that uses any of the aggregation mechanisms (spelled out by 18.3.4.1 Grouping and Aggregation as the use of GROUP BY or "the use of aggregates in HAVING or ORDER BY clauses, or in the projection").

changes the language about the restriction on expressions from only "consisting of aggregates and constants" or simple GROUP BY variables to what I think is more explicit language about conditions that every variable used in SELECT expressions in these aggregating queries MUST satisfy

explicitly state that if the conditions on variables in SELECT expressions are not satisfied that the query is syntactically invalid

Closes #377

Preview | Diff

Closes #377

Co-authored-by: Ruben Taelman <rubensworks@users.noreply.github.com>

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

kasei · 2026-05-27T14:24:29Z

Thanks @hartig. Very good suggestions. I committed all but one, which I commented on directly.

hartig · 2026-05-27T15:15:38Z

+        <p>In a query level which uses grouping (either by the explicit use of a <code>GROUP BY</code> clause
+          or through the use of aggregates in projection, <code>HAVING</code>, or <code>ORDER BY</code> clauses),
+          every variable that appears in projection or SELECT expressions of that query level
+          MUST satisfy exactly one of the following conditions:</p>


Now that I understand what the first bullet point is about, I would say that "exactly one" is too restrictive here. It would also be okay if a variable satisfies both the first and the second condition. Likewise, it would be okay if a variable satisfies both the second and the third condition. It may even be the case that a variable satisfies the first condition in multiple different ways (by being an argument to multiple aggregates within the corresponding SELECT clause).

Having said that, I don't think we can simply replace "exactly one" by "at least one" because, then, a query such as the following would be permitted (because ?x satisfies the first condition).

SELECT ?x (MIN(?x) AS ?y) { # ... ?z ... } GROUP BY ?z

The underlying issue is that the conditions are actually not about the variables per se, but about (all) the occurrences of the variables within the SELECT clause.

Agreed it's about occurrences, not the variables themselves. I'm hesitant to get to into the weeds with this language, but I'll think about how we might tweak it to talk about occurrences.

Perhaps it's as simple as introducing "occurrence":

every occurrence of a variable that appears in projection or SELECT expressions of that query level MUST satisfy exactly one of the following conditions:
…
If such a variable occurrence does not satisfy one of these conditions, the query is syntactically invalid.

?

Yes! Introducing "occurrence" in this way works, together with just saying "one of" instead of saying "exactly one of."

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

hartig

Looks good now!

Co-authored-by: Andy Seaborne <andy@apache.org>

…ECT expression See w3c/sparql-query#380

TallTed · 2026-05-29T22:30:21Z

+        <p>In a query level which uses grouping (either by the explicit use of a <code>GROUP BY</code> clause
+          or through the use of aggregates in projection, <code>HAVING</code>, or <code>ORDER BY</code> clauses),
+          every occurrence of a variable that appears in projection or SELECT expressions of that query level
+          MUST satisfy one of the following conditions:</p>


Suggested change

<p>In a query level which uses grouping (either by the explicit use of a <code>GROUP BY</code> clause

or through the use of aggregates in projection, <code>HAVING</code>, or <code>ORDER BY</code> clauses),

every occurrence of a variable that appears in projection or SELECT expressions of that query level

MUST satisfy one of the following conditions:</p>

<p>In a query level which uses grouping (either by the explicit use of a <code>GROUP BY</code> clause,

or by the use of an aggregate in projection, a <code>HAVING</code> clause, or an <code>ORDER BY</code> clause),

every occurrence of a variable that appears in a projection or a SELECT expression of that query level

MUST satisfy one of the following conditions:</p>

@TallTed – This has already been merged. A new PR is probably the best approach for this change.

moved to issue #386 and pr #387

The queryProjectionIsGood function in rules-sparql-1-2/validators.ts is a separate implementation from the 1.1 version (changed per w3c/sparql-query#380). Although all 1.1 tests correctly run against the 1.2 parser, several branches of this function were not reached by any existing test. Fix by adding cases to the shared test infrastructure so they run on both parsers automatically: - 3 new sparql-1-1-invalid static queries (select-star-with-group-by, ungrouped-variable-in-expression-projection, count-with-ungrouped-expression-variable) that cover the throw paths in queryProjectionIsGood (lines 123 and 158). - 3 new note tests in importSparql11NoteTests covering the no-throw branches: AS variable not conflicting with a term-var subquery, AS variable not conflicting with a wildcard subquery, and expression projection variable covered by GROUP BY (line 158 and 183-186 FALSE branches). Also add rapport.md documenting the investigation findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* feat: add select-variable-reuse test and allow AS-bound variable references Add support for referencing variables bound by preceding (expr AS ?var) expressions in subsequent SELECT expressions, per the updated SPARQL spec (w3c/sparql-query#380, w3c/rdf-tests#340). The query `SELECT (COUNT(?v) AS ?count) (?count + 1 AS ?countPlusOne) ...` was previously rejected with 'Use of ungrouped variable' because the validator did not recognize that ?count was in scope from an earlier AS binding. Changes: - Fix queryProjectionIsGood validation to track AS-bound variables and allow their use in subsequent projection expressions - Add select-variable-reuse test with AST, algebra, and generator statics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * some comments * Split 1.1 and 1.2 validators * test: achieve 100% coverage on rules-sparql-1-2 validators The queryProjectionIsGood function in rules-sparql-1-2/validators.ts is a separate implementation from the 1.1 version (changed per w3c/sparql-query#380). Although all 1.1 tests correctly run against the 1.2 parser, several branches of this function were not reached by any existing test. Fix by adding cases to the shared test infrastructure so they run on both parsers automatically: - 3 new sparql-1-1-invalid static queries (select-star-with-group-by, ungrouped-variable-in-expression-projection, count-with-ungrouped-expression-variable) that cover the throw paths in queryProjectionIsGood (lines 123 and 158). - 3 new note tests in importSparql11NoteTests covering the no-throw branches: AS variable not conflicting with a term-var subquery, AS variable not conflicting with a wildcard subquery, and expression projection variable covered by GROUP BY (line 158 and 183-186 FALSE branches). Also add rapport.md documenting the investigation findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: move 3 note tests from Sparql11NotesTest to static files Replace the three programmatic 'should NOT throw' tests added in the previous commit with proper static test files that slot into the shared positive-test infrastructure and therefore run automatically on all parsers (1.1, 1.1-adjust, 1.2) and the 1.2 generator. Files added per test case: - sparql/sparql-1-1/<name>.sparql – the query under test - ast-source-tracked/sparql-1-1/<name>.json – expected AST (source-tracked) - sparql-generated/sparql-1-1/<name>.sparql – expected generator output - sparql-generated-compact/sparql-1-1/<name>.sparql – compact generator output Cases migrated: - note-as-var-not-in-term-subquery - note-as-var-not-in-wildcard-subquery - note-expression-grouped-var Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * No rapport --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Update Aggregate Projection Restrictions.

bcbc357

Closes #377

kasei requested review from Tpt, afs, hartig and rubensworks May 22, 2026 16:10

rubensworks approved these changes May 23, 2026

View reviewed changes

Comment thread spec/index.html Outdated

Tpt approved these changes May 23, 2026

View reviewed changes

Tpt mentioned this pull request May 23, 2026

SPARQL 1.2: Add a test to cover variable introduced by an earlier SELECT expression w3c/rdf-tests#340

Merged

Update spec/index.html

97b33aa

Co-authored-by: Ruben Taelman <rubensworks@users.noreply.github.com>

jitsedesmet mentioned this pull request May 27, 2026

feat: allow AS-bound variable references comunica/traqula#142

Merged

hartig requested changes May 27, 2026

View reviewed changes

Comment thread spec/index.html Outdated

Comment thread spec/index.html Outdated

Comment thread spec/index.html Outdated

Comment thread spec/index.html Outdated

Comment thread spec/index.html Outdated

kasei and others added 4 commits May 27, 2026 07:16

Update spec/index.html

24ba680

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

Update spec/index.html

309f6d8

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

Update spec/index.html

535afcb

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

Update spec/index.html

2b4f53f

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

hartig reviewed May 27, 2026

View reviewed changes

kasei and others added 2 commits May 27, 2026 08:54

Update spec/index.html

378850e

Co-authored-by: Olaf Hartig <olaf.hartig@liu.se>

Update Aggregate Projection Restrictions language based on PR feedback.

da20e74

hartig approved these changes May 27, 2026

View reviewed changes

afs reviewed May 28, 2026

View reviewed changes

Comment thread spec/index.html

afs approved these changes May 28, 2026

View reviewed changes

Update spec/index.html

ba816b8

Co-authored-by: Andy Seaborne <andy@apache.org>

kasei merged commit 277a285 into main May 28, 2026
2 checks passed

Tpt pushed a commit to w3c/rdf-tests that referenced this pull request May 28, 2026

SPARQL 1.2: Add a test to cover variable introduced by an earlier SEL…

8a4b7fc

…ECT expression See w3c/sparql-query#380

TallTed reviewed May 29, 2026

View reviewed changes

Uh oh!

Conversation

kasei commented May 22, 2026 • edited by pr-preview Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kasei commented May 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hartig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kasei commented May 22, 2026 •

edited by pr-preview Bot

Loading