Feature/381 sparql 11 variable visibility by remiceres · Pull Request #454 · corese-stack/corese-core

remiceres · 2026-05-28T10:11:06Z

Implemented for issue #381.

Added variable visibility validation for GROUP BY
Added variable visibility validation for HAVING
Added variable visibility validation for SELECT (expr AS ?var)
Added support for immediate alias visibility inside the same SELECT clause
Added ORDER BY scope validation for ASK queries
Extended non-SELECT ORDER BY scope validation to include VALUES
Added grouped SELECT projection validation:
- reject projected variables that are not grouped and not aggregated
- reject projection expressions that use non-grouped variables outside aggregates
Added implicit aggregate query validation when there is no explicit GROUP BY
Rejected SELECT * when GROUP BY is present
Extended the AST to support GROUP BY (expr AS ?alias)
Added validation support for GROUP BY expression aliases in:
- SELECT
- HAVING
- ORDER BY
Added grouped HAVING validation:
- reject variables used outside aggregates if they are not grouped
Added grouped ORDER BY validation:
- reject variables used outside aggregates if they are not grouped
Added validation coverage for projected aggregate aliases used in ORDER BY
Refactored common semantic validation helpers to avoid duplication across grouped rules
Added and updated targeted parser and validation tests for all the cases above

github-actions · 2026-05-28T10:13:35Z

Overall Project	47.98% `-0.1%`	🍏
Files changed	79.61%	🍏

File	Coverage
SelectProjectionScopeValidationRule.java	100%	🍏
HavingScopeValidationRule.java	100%	🍏
GroupByScopeValidationRule.java	100%	🍏
ScopeClause.java	100%	🍏
OrderByScopeValidationRule.java	100%	🍏
GroupedOrderByValidationRule.java	98.24% `-1.76%`	🍏
GroupedSelectProjectionValidationRule.java	98.17% `-1.83%`	🍏
AbstractSemanticValidationRule.java	97.17% `-2.83%`	🍏
GroupedHavingValidationRule.java	96.43% `-3.57%`	🍏
SelectQueryFeature.java	94.74%	🍏
GroupByAst.java	93.02% `-4.65%`	🍏
SparqlQuerySemanticValidator.java	92.42%	🍏
SolutionModifierFeature.java	90.8% `-4.91%`	🍏
SparqlAstBuilder.java	83.39% `-0.8%`	🍏
ProjectionAst.java	81.82% `-2.6%`	🍏
VariableScopeAnalyzer.java	69.4% `-24.36%`	❌
ProjectionAsts.java	54.1% `-31.15%`	❌

github-actions · 2026-05-28T10:13:49Z

Test Results

414 files 414 suites 19s ⏱️
2 293 tests 2 293 ✅ 0 💤 0 ❌
2 307 runs 2 307 ✅ 0 💤 0 ❌

Results for commit 9ad9945.

♻️ This comment has been updated with latest results.

github-actions · 2026-05-28T13:22:25Z

Overall Project	47.99% `-0.09%`	🍏
Files changed	81.47%	🍏

File	Coverage
SelectProjectionScopeValidationRule.java	100%	🍏
HavingScopeValidationRule.java	100%	🍏
GroupByScopeValidationRule.java	100%	🍏
ScopeClause.java	100%	🍏
OrderByScopeValidationRule.java	100%	🍏
GroupedOrderByValidationRule.java	98.86% `-1.14%`	🍏
GroupedSelectProjectionValidationRule.java	98.17% `-1.83%`	🍏
AbstractSemanticValidationRule.java	97.17% `-2.83%`	🍏
GroupedHavingValidationRule.java	96.43% `-3.57%`	🍏
SelectQueryFeature.java	94.74%	🍏
GroupByAst.java	93.02% `-4.65%`	🍏
SparqlQuerySemanticValidator.java	92.42%	🍏
SolutionModifierFeature.java	90.8% `-4.91%`	🍏
SparqlAstBuilder.java	83.39% `-0.8%`	🍏
ProjectionAst.java	81.82% `-2.6%`	🍏
VariableScopeAnalyzer.java	71.82% `-21.94%`	❌
ProjectionAsts.java	65.57% `-31.15%`	❌

github-actions · 2026-05-28T13:39:02Z

Overall Project	47.99% `-0.09%`	🍏
Files changed	81.47%	🍏

File	Coverage
SelectProjectionScopeValidationRule.java	100%	🍏
HavingScopeValidationRule.java	100%	🍏
GroupByScopeValidationRule.java	100%	🍏
ScopeClause.java	100%	🍏
OrderByScopeValidationRule.java	100%	🍏
GroupedOrderByValidationRule.java	98.86% `-1.14%`	🍏
GroupedSelectProjectionValidationRule.java	98.17% `-1.83%`	🍏
AbstractSemanticValidationRule.java	97.17% `-2.83%`	🍏
GroupedHavingValidationRule.java	96.43% `-3.57%`	🍏
SelectQueryFeature.java	94.74%	🍏
GroupByAst.java	93.02% `-4.65%`	🍏
SparqlQuerySemanticValidator.java	92.42%	🍏
SolutionModifierFeature.java	90.8% `-4.91%`	🍏
SparqlAstBuilder.java	83.39% `-0.8%`	🍏
ProjectionAst.java	81.82% `-2.6%`	🍏
VariableScopeAnalyzer.java	71.82% `-21.94%`	❌
ProjectionAsts.java	65.57% `-31.15%`	❌

MaillPierre

I have some comments before approving because I'm lost by some changes.

MaillPierre · 2026-05-28T14:23:54Z

+        QueryAst ast = parser.parse("""
+                SELECT * WHERE {
+                    ?s ?p ?o .
+                    BIND(IF(BOUND(?s), COUNT(?o), ?p) AS ?result)


Can you explain this test please ? I don't understand where the aggregate is.

Here the aggregate is COUNT(?o) inside the IF(...) expression.

This test is here to check collectReferencedVariablesOutsideAggregates(...)

The query itself does not really make sense as a full example.
It is only used to build a AST with some variables outside an aggregate and one variable inside an aggregate

The only part really used by the test is:
BIND(IF(BOUND(?s), COUNT(?o), ?p) AS ?result)

In this expression:

?s is outside the aggregate

?p is outside the aggregate

?o is inside COUNT(?o)

So the helper should keep ?s and ?p, and ignore ?o

MaillPierre · 2026-05-28T14:25:27Z


        QueryAst ast = parser.parse("""
-                SELECT ?s WHERE {
+                SELECT (AVG(?val) AS ?avg) WHERE {


Why change that ? AVG(?val) appears twice in the query now.

I changed it because the old query was not valid

In the old version SELECT ?s ... ORDER BY AVG(?val)
AVG(?val) makes the query an aggregate query, but ?s is not grouped and not aggregated.

The new version keeps the same thing we want to test (ORDER BY AVG(?val)), but also makes the full query valid: SELECT (AVG(?val) AS ?avg) ... ORDER BY AVG(?val)

So yes, AVG(?val) appears twice, once to make the query valid and once because this is the ORDER BY expression we want to parse

MaillPierre · 2026-05-28T14:29:22Z

+                WHERE {
+                    ?s ?p ?o .
+                }
+                ORDER BY ?s


This is grammatically correct ... It does not make any sense, though. Only HAVING and LIMIT would impact the query execution.
This is just a comment.

Yes, I agree.

This test is mainly here for parser/AST consistency.
It is not here because ORDER BY is especially useful for ASK.

I first had this kind of check for SELECT, and in this PR I extended it to the other query forms too, to keep the behavior consistent.

So here the goal is only to check that ORDER BY is parsed and stored correctly in the AST for ASK.

prbblrypier · 2026-06-02T15:21:50Z

+
+    private void validateProjectionExpression(
+            String projectionVariableName,
+            ProjectionAst projection,


add GROUP BY.

Set<String> groupedVariables,

prbblrypier · 2026-06-02T15:22:44Z

+            return;
+        }
+        for (String referencedVariable : collectReferencedVariablesOutsideAggregates(expression)) {
+            diagnostics.add(buildGroupedProjectionDiagnostic(referencedVariable));


if (!groupedVariables.contains(referencedVariable)) { diagnostics.add(buildGroupedProjectionDiagnostic(referencedVariable)); }

prbblrypier · 2026-06-02T15:25:26Z

we can add the following test :

@Test @DisplayName("Should accept grouped SELECT expression using a variable from GROUP BY ?var") void shouldAcceptGroupedSelectExpressionUsingGroupedVariable() { SparqlParser parser = newParserDefault(); QueryAst ast = parser.parse(""" SELECT ?groupedVar (STR(?groupedVar) AS ?alias) WHERE { ?groupedVar ?p ?o } GROUP BY ?groupedVar """); SelectQueryAst selectQueryAst = assertInstanceOf(SelectQueryAst.class, ast); ProjectionAst projection = selectQueryAst.projection(); assertFalse(projection.selectAll()); assertEquals(2, projection.variables().size()); assertEquals("groupedVar", projection.variables().get(0).name()); assertEquals("alias", projection.variables().get(1).name()); assertTrue(projection.expressionBoundVariables().contains("alias")); }

remiceres added 15 commits May 26, 2026 16:17

feat(next/parser): validate GROUP BY variable visibility

1fe99ee

feat(next/parser): validate HAVING variable visibility

c2e8547

feat(next/parser): validate SELECT expression variable visibility

4f9099e

refactor(next/parser): centralize semantic scope helpers

72ca6fe

feat(next/parser): validate ORDER BY scope for ASK queries

04b2aca

feat(next/parser): support immediate SELECT alias scope

130e65a

feat(next/parser): validate grouped SELECT projections

1867543

feat(next/parser): validate implicit aggregate projections

db6ce68

feat(next/parser): reject SELECT * with GROUP BY

119e4f4

feat(next/parser): support GROUP BY expression aliases

6bb8d10

feat(next/parser): support HAVING on GROUP BY aliases

720b026

feat(next/parser): support ORDER BY on GROUP BY aliases

30f331a

feat(next/parser): validate grouped HAVING expressions

a59d9d0

feat(next/parser): validate grouped ORDER BY expressions

952f798

test(next/parser): cover projected aggregate aliases in ORDER BY

709d662

test(next/parser): polish semantic helper coverage

550329c

ci: avoid earlier-commit lookup in test result publishing

9ad9945

remiceres requested a review from prbblrypier May 28, 2026 13:40

remiceres self-assigned this May 28, 2026

remiceres requested a review from MaillPierre May 28, 2026 13:41

remiceres added the Refactoring Issue created during the 2025 refactoring effort label May 28, 2026

MaillPierre reviewed May 28, 2026

View reviewed changes

MaillPierre approved these changes Jun 1, 2026

View reviewed changes

prbblrypier requested changes Jun 2, 2026

View reviewed changes

Conversation

remiceres commented May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

MaillPierre left a comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented May 28, 2026 •

edited

Loading