Skip to content

repl: Allow interactive ref head rule definitions - #9001

Merged
sspaink merged 1 commit into
open-policy-agent:mainfrom
sspaink:repl-ref-head-rule-definitions
Aug 13, 2026
Merged

repl: Allow interactive ref head rule definitions#9001
sspaink merged 1 commit into
open-policy-agent:mainfrom
sspaink:repl-ref-head-rule-definitions

Conversation

@sspaink

@sspaink sspaink commented Aug 11, 2026

Copy link
Copy Markdown
Member

Fixes: #5498

Statements such as a[0] := 1 or p.q.r := 1 were rejected with "cannot assign to ref". The REPL refused to interpret refs of more than one term as rule heads, so those statements fell through to being compiled as a query body, where assigning to a ref isn't allowed.

They are now interpreted as rule definitions. Refs rooted at data or input are excluded, as data.foo.bar = 1 asks whether that document is 1 rather than defining a rule.

Rules are identified by their head ref instead of Head.Name when unsetting them, which fixes two related problems: ref head rules (so far only definable with the if keyword) could not be unset at all, because their Head.Name is empty, and re-defining a[0] := 1 doesn't drop unrelated keys of the same document, e.g. a[1]. As a consequence, unset accepts a ref, e.g. unset a[0] or unset p.q.r, and removes all rules below it.

ParsePartialObjectDocRuleFromEqExpr didn't mark the true body it generates as generated, so rules parsed from these statements tripped the rego-v1 check requiring if before a rule body. The module parser sets that flag itself after calling ParseRuleFromBody, which is why this only surfaced for direct callers like the REPL.

Comment thread v1/repl/repl.go Outdated
//
// > a[0] := 1
// > a
// {"0": 1}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be that a user["role"] := "admin" demonstrates a more real use case for this. Just jumped out to me that this is odd to use an example here that looks like an array, but becomes an obj 'because'.

@charlieegan3

Copy link
Copy Markdown
Contributor

Tested myself and it all seems to work ok.

Statements such as `a[0] := 1` or `p.q.r := 1` were rejected with
"cannot assign to ref". The REPL refused to interpret refs of more than
one term as rule heads, so those statements fell through to being
compiled as a query body, where assigning to a ref isn't allowed.

They are now interpreted as rule definitions. Refs rooted at data or
input are excluded, as `data.foo.bar = 1` asks whether that document is
1 rather than defining a rule.

Rules are identified by their head ref instead of Head.Name when
unsetting them, which fixes two related problems: ref head rules (so
far only definable with the `if` keyword) could not be unset at all,
because their Head.Name is empty, and re-defining `a[0] := 1` doesn't
drop unrelated keys of the same document, e.g. a[1]. As a consequence,
`unset` accepts a ref, e.g. `unset a[0]` or `unset p.q.r`, and removes
all rules below it.

ParsePartialObjectDocRuleFromEqExpr didn't mark the `true` body it
generates as generated, so rules parsed from these statements tripped
the rego-v1 check requiring `if` before a rule body. The module parser
sets that flag itself after calling ParseRuleFromBody, which is why
this only surfaced for direct callers like the REPL.

Fixes: open-policy-agent#5498
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
@sspaink
sspaink force-pushed the repl-ref-head-rule-definitions branch from 74bf9fd to 578fa9e Compare August 13, 2026 17:07
@sspaink
sspaink enabled auto-merge (squash) August 13, 2026 17:07
@sspaink
sspaink merged commit ee937f6 into open-policy-agent:main Aug 13, 2026
43 checks passed
@github-actions

Copy link
Copy Markdown
Benchmark Comparison (`8b1b42ee113bfe8bb91609eec951cd3d79c387a3` vs `ee937f696ee0bde8146ade3580687fbd5651331c`)
benchmark delta
GenerateLocalVar -1.93%
RuleIndexRefOrdering/frequency-descending +1.03%
RuleIndexRefOrdering/unordered +1.53%
ParseModuleRulesBase/1 +3.77%
ParseModuleRulesBase/10 +4.96%
ParseModuleRulesBase/100 +4.55%
ParseModuleRulesBase/1000 +3.46%
ParseStatementBasicCall +3.75%
ParseStatementMixedJSON +3.77%
ParseStatementSimpleArray/1 +3.56%
ParseStatementSimpleArray/10 +2.51%
ParseStatementSimpleArray/100 +3.91%
ParseStatementSimpleArray/1000 +4.79%
ParseStatementNestedObjects/1x1 +3.64%
ParseStatementNestedObjects/5x1 +4.26%
ParseStatementNestedObjects/10x1 +3.74%
ParseStatementNestedObjects/1x5 +4.65%
ParseStatementNestedObjects/1x10 +4.58%
ParseStatementNestedObjects/5x5 +4.73%
ParseSome +3.20%
ParseEvery +3.42%
ParseDeepNesting/NestedArrays/depth-10 +5.00%
ParseDeepNesting/NestedArrays/depth-50 +4.74%
ParseDeepNesting/NestedArrays/depth-100 +6.35%
ParseDeepNesting/NestedArrays/depth-500 +5.98%
ParseDeepNesting/NestedArrays/depth-2500 +5.87%
ParseDeepNesting/NestedObjects/depth-10 +4.01%
ParseDeepNesting/NestedObjects/depth-50 +3.05%
ParseDeepNesting/NestedObjects/depth-100 +3.95%
ParseDeepNesting/NestedObjects/depth-500 +4.54%
ParseDeepNesting/NestedObjects/depth-2500 +4.12%
ParseDeepNesting/NestedObjects/depth-12500 +3.43%
ParseStatementNestedObjectsOrSets/1 +2.73%
ParseStatementNestedObjectsOrSets/5 +3.47%
ParseStatementNestedObjectsOrSets/10 +2.09%
ParseStatementNestedObjectsOrSets/15 +2.82%
ParseStatementNestedObjectsOrSets/20 +2.99%
ParseVars +3.40%
ParseBasicABACModule +3.96%
ParseManyAnnotations +3.01%
CommentString +2.20%
ExprString/simple_expr -3.30%
ExprString/negated_expr_with_with_modifier -2.98%
RuleMarshalJSON/rule_with_multiple_exprs +2.00%
TypeName +14.81%
ObjectFind/50_5 +6.33%
ObjectCreateWithInsertVsTermsArray/insert +3.48%
LazyObjectLookup/5 +1.49%
LazyObjectLookup/50 +1.73%
LazyObjectLookup/500 +1.51%
LazyObjectFind/5_50 -6.34%
LazyObjectFind/5_500 -6.44%
LazyObjectFind/5_5000 -12.24%
LazyObjectFind/50_50 -6.28%
LazyObjectFind/50_500 -6.48%
LazyObjectFind/50_5000 -11.93%
LazyObjectFind/500_50 -5.84%
LazyObjectFind/500_500 -6.45%
LazyObjectFind/500_5000 -12.12%
LazyObjectFind/5000_50 -6.17%
LazyObjectFind/5000_5000 -11.93%
SetIntersection/5 +1.22%
SetIntersection/50 +2.03%
SetUnion/5 +3.34%
SetUnionOverlapping/50 +0.94%
ObjectCopy +1.42%
ObjectStringInterfaces/5/String() -2.90%
ObjectConstruction/increasing_keys/5 -1.79%
ObjectConstruction/increasing_keys/50 -1.57%
ArrayString/500/String() -2.67%
IsVarCompatibleString/#00 -22.24%
IsVarCompatibleString/ello -24.59%
IsVarCompatibleString/h_llo -18.05%
IsVarCompatibleString/hello -24.28%
RefString/var_term -14.23%
RefString/with_escape -10.73%
InterfaceToValueInt/non-interned_int_value +11.38%
Ptr/with_escape -4.41%
Ptr/without_escape -10.38%
EscapeTemplateStringStringPart/_*_100 +10.51%
TemplateStringToString -2.74%
TemplateStringToStringEscapeControl -3.62%
FromBuiltinNames/no_match_long -2.51%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/string_with_newlines +0.38%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/null +4.79%

This comment was automatically generated by the benchmarks workflow.

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.

OPA REPL compile error for array refs

2 participants