Skip to content

Fix/dag scope #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
noahboerger wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Fix/dag scope #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83477be
fix: PersistentDataset delegate to SparkDataset
noahboerger Jan 19, 2026
58bf0fc
fix: DAG scopes #457
noahboerger Jan 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 100 additions & 23 deletions vtl-engine/src/main/java/fr/insee/vtl/engine/visitors/DAGBuildingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.antlr.v4.runtime.RuleContext;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.tree.RuleNode;
import org.antlr.v4.runtime.tree.TerminalNode;
Expand Down Expand Up @@ -154,10 +155,10 @@ protected List<DAGStatement> defaultResult() {
* <code>VarIDsExtractingVisitor</code> is the visitor for extracting the used VarIds from VTL
* statements.
*/
private static class IdentifierExtractingVisitor
extends VtlBaseVisitor<Set<DAGStatement.Identifier>> {
static class IdentifierExtractingVisitor extends VtlBaseVisitor<Set<DAGStatement.Identifier>> {

private final Set<String> ignoreInnerScopedVarIdentifiers;
private int componentContextDepth = 0;

public IdentifierExtractingVisitor() {
this(Set.of());
Expand All @@ -168,51 +169,127 @@ public IdentifierExtractingVisitor(Set<String> ignoreInnerScopedVarIdentifiers)
this.ignoreInnerScopedVarIdentifiers = ignoreInnerScopedVarIdentifiers;
}

private Set<DAGStatement.Identifier> enterRestrictedContext(RuleContext ctx) {
componentContextDepth++;
Set<DAGStatement.Identifier> result = super.visitChildren(ctx);
componentContextDepth--;
return result;
}

@Override
public Set<DAGStatement.Identifier> visitVarID(VtlParser.VarIDContext node) {
final var currentVarIdentifier = node.IDENTIFIER().getSymbol().getText();
final Set<DAGStatement.Identifier> thisResult =
ignoreInnerScopedVarIdentifiers.contains(currentVarIdentifier)
? Set.of()
: Set.of(
new DAGStatement.Identifier(
DAGStatement.Identifier.Type.VARIABLE, currentVarIdentifier));
final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
return aggregateResult(thisResult, subResult);

// If we are inside a component context (depth > 0), we ignore this identifier
// because it is a component name, not a dataset dependency, according to the unadjusted VTL
// syntax
if (componentContextDepth > 0) {
return Set.of();
}

return ignoreInnerScopedVarIdentifiers.contains(currentVarIdentifier)
? Set.of()
: Set.of(
new DAGStatement.Identifier(
DAGStatement.Identifier.Type.VARIABLE, currentVarIdentifier));
}

// Workaround for https://github.com/InseeFr/Trevas/issues/457, as long the open points in here
// are not clarified and https://github.com/InseeFr/Trevas/issues/355 is not implemented
// If we are inside a component context (depth > 0), we ignore this identifier
// because it is a component name, not a dataset dependency, according to the unadjusted VTL
// syntax
@Override
public Set<DAGStatement.Identifier> visitOperatorID(VtlParser.OperatorIDContext node) {
final Set<DAGStatement.Identifier> thisResult =
Set.of(
new DAGStatement.Identifier(
DAGStatement.Identifier.Type.OPERATOR, node.IDENTIFIER().getSymbol().getText()));
final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
return aggregateResult(thisResult, subResult);
public Set<DAGStatement.Identifier> visitFilterClause(VtlParser.FilterClauseContext ctx) {
return enterRestrictedContext(ctx);
}

@Override
public Set<DAGStatement.Identifier> visitCalcClauseItem(VtlParser.CalcClauseItemContext ctx) {
return enterRestrictedContext(ctx);
}

@Override
public Set<DAGStatement.Identifier> visitHavingClause(VtlParser.HavingClauseContext ctx) {
return enterRestrictedContext(ctx);
}

@Override
public Set<DAGStatement.Identifier> visitJoinApplyClause(VtlParser.JoinApplyClauseContext ctx) {
return enterRestrictedContext(ctx);
}

@Override
public Set<DAGStatement.Identifier> visitAggrFunctionClause(
VtlParser.AggrFunctionClauseContext ctx) {
return enterRestrictedContext(ctx);
}

// Aggregate & Analytic Functions (First arg is Dataset, rest are components)
@Override
public Set<DAGStatement.Identifier> visitAggrDataset(VtlParser.AggrDatasetContext ctx) {
Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
return aggregateResult(datasetRef, enterRestrictedContext(ctx));
}

@Override
public Set<DAGStatement.Identifier> visitAnSimpleFunction(
VtlParser.AnSimpleFunctionContext ctx) {
Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
return aggregateResult(datasetRef, enterRestrictedContext(ctx));
}

@Override
public Set<DAGStatement.Identifier> visitLagOrLeadAn(VtlParser.LagOrLeadAnContext ctx) {
Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
return aggregateResult(datasetRef, enterRestrictedContext(ctx));
}

@Override
public Set<DAGStatement.Identifier> visitRatioToReportAn(VtlParser.RatioToReportAnContext ctx) {
Set<DAGStatement.Identifier> datasetRef = visit(ctx.expr());
return aggregateResult(datasetRef, enterRestrictedContext(ctx));
}

@Override
public Set<DAGStatement.Identifier> visitRankAn(VtlParser.RankAnContext ctx) {

return enterRestrictedContext(ctx);
}

@Override
public Set<DAGStatement.Identifier> visitMembershipExpr(VtlParser.MembershipExprContext ctx) {
// Only visit the dataset (left side), ignore the component (right side)
return visit(ctx.expr());
}

@Override
public Set<DAGStatement.Identifier> visitValidateDPruleset(
VtlParser.ValidateDPrulesetContext node) {
final Set<DAGStatement.Identifier> thisResult =
Set<DAGStatement.Identifier> rulesetRef =
Set.of(
new DAGStatement.Identifier(
DAGStatement.Identifier.Type.RULESET_DATAPOINT,
node.IDENTIFIER().getSymbol().getText()));
final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
return aggregateResult(thisResult, subResult);
return aggregateResult(rulesetRef, visit(node.expr()));
}

@Override
public Set<DAGStatement.Identifier> visitValidateHRruleset(
VtlParser.ValidateHRrulesetContext node) {
final Set<DAGStatement.Identifier> thisResult =
Set<DAGStatement.Identifier> rulesetRef =
Set.of(
new DAGStatement.Identifier(
DAGStatement.Identifier.Type.RULESET_HIERARCHICAL,
node.IDENTIFIER().getSymbol().getText()));
final Set<DAGStatement.Identifier> subResult = this.visitChildren(node);
return aggregateResult(thisResult, subResult);
return aggregateResult(rulesetRef, visit(node.expr()));
}

@Override
public Set<DAGStatement.Identifier> visitOperatorID(VtlParser.OperatorIDContext node) {
return Set.of(
new DAGStatement.Identifier(
DAGStatement.Identifier.Type.OPERATOR, node.IDENTIFIER().getSymbol().getText()));
}

@Override
Expand Down
18 changes: 12 additions & 6 deletions vtl-engine/src/test/java/fr/insee/vtl/engine/utils/dag/DagDefineStatementsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import fr.insee.vtl.model.exceptions.VtlScriptException;
import fr.insee.vtl.parser.VtlLexer;
import fr.insee.vtl.parser.VtlParser;
import java.util.*;
import java.util.Set;
import java.util.stream.Stream;
import javax.script.ScriptException;
import org.antlr.v4.runtime.*;
import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CodePointCharStream;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.RuleNode;
import org.antlr.v4.runtime.tree.TerminalNode;
Expand All @@ -21,15 +23,19 @@ public class DagDefineStatementsTest {

private static String performDagReordering(final String script, final Set<String> bindingVars)
throws VtlScriptException {
final CodePointCharStream stream = CharStreams.fromString(script);
VtlLexer lexer = new VtlLexer(stream);
VtlParser parser = new VtlParser(new CommonTokenStream(lexer));
var start = parser.start();
VtlParser.StartContext start = parseScript(script);
VtlSyntaxPreprocessor syntaxPreprocessor = new VtlSyntaxPreprocessor(start, bindingVars);
VtlParser.StartContext res = syntaxPreprocessor.checkForMultipleAssignmentsAndReorderScript();
return parseTreeToText(res);
}

public static VtlParser.StartContext parseScript(String script) {
final CodePointCharStream stream = CharStreams.fromString(script);
VtlLexer lexer = new VtlLexer(stream);
VtlParser parser = new VtlParser(new CommonTokenStream(lexer));
return parser.start();
}

private static String parseTreeToText(ParseTree child) {
StringBuilder result = new StringBuilder();
if (child instanceof TerminalNode) {
Expand Down
2 changes: 0 additions & 2 deletions vtl-engine/src/test/java/fr/insee/vtl/engine/utils/dag/DagTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,6 @@ void testMultipleCycles() {
void testDagDoubleAssignment() {
ScriptContext context = engine.getContext();
context.setAttribute("a", 1L, ScriptContext.ENGINE_SCOPE);
// Note that the double assignment is not detected while building the DAG but later during
// execution
assertThatThrownBy(() -> engine.eval("b := a; b := 1;"))
.isInstanceOf(VtlScriptException.class)
.hasMessage("Dataset b has already been assigned");
Expand Down
Loading
Loading