Skip to content

fix DotExpr for fusions and improve missing field semantics - #7297

Merged
mccanne merged 2 commits into
mainfrom
dot-fusion-none
Sep 11, 2026
Merged

fix DotExpr for fusions and improve missing field semantics#7297
mccanne merged 2 commits into
mainfrom
dot-fusion-none

Conversation

@mccanne

@mccanne mccanne commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

This commit makes DotExpr's work with fusion and nones. There is currently a slow-path defuse whenever a none value or missing-field error is encountered. We can improve this later.

In making these changes, we realized it would be better semantics for a reference to a non-existent field always return an error even for the "?." operator. Instead, ok() with a none operator should be used when missing fields are expected.

Ok-pushdown will be added in a subsequent PR where the defuse step required to create the structured error can be avoided when we know the dot operation is wrapped in an ok() or is_ok(). We added the flag to the DotExpr struct but still need to wire it up.

This commit makes DotExpr's work with fusion and nones.  There is
currently a slow-path defuse whenever a none value or missing-field
error is encountered.  We can improve this later.

In making these changes, we realized it would be better semantics for
a reference to a non-existent field always return an error even for
the "?." operator.  Instead, ok() with a none operator should be used
when missing fields are expected.

Ok-pushdown will be added in a subsequent PR where the defuse step
required to create the structured error can be avoided when we know
the dot operation is wrapped in an ok() or is_ok().  We added the
flag to the DotExpr struct but still need to wire it up.
Comment thread runtime/vam/expr/dot.go Outdated
Comment on lines +114 to +125
if _, ok := vec.(*vector.None); ok {
return true
}
if vec, ok := vec.(*vector.Fusion); ok {
return hasNone(vec.Values)
}
if vec, ok := vec.(*vector.Dynamic); ok {
return slices.IndexFunc(vec.Values, hasNone) >= 0
}
if super.IsOptionType(vec.Type()) {
return hasNone(vec.(*vector.Union).Dynamic())
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Use a type switch.

Suggested change
if _, ok := vec.(*vector.None); ok {
return true
}
if vec, ok := vec.(*vector.Fusion); ok {
return hasNone(vec.Values)
}
if vec, ok := vec.(*vector.Dynamic); ok {
return slices.IndexFunc(vec.Values, hasNone) >= 0
}
if super.IsOptionType(vec.Type()) {
return hasNone(vec.(*vector.Union).Dynamic())
}
switch vec := vec.(type) {
case *vector.None:
return true
case *vector.Union:
return super.IsOptionType(vec.Type()) && hasNone(vec.Dynamic())
case *vector.Fusion:
return hasNone(vec.Values)
case *vector.Dynamic:
return slices.IndexFunc(vec.Values, hasNone) >= 0
}

Comment thread runtime/vam/expr/dot.go
entity Evaluator
key string
noneish bool
okPush bool

@nwt nwt Sep 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: okPush is a pretty opaque name. Maybe call it something more intention-revealing, like skipMissingDefuse?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's ok pushdown. What rather than how

@mccanne
mccanne merged commit 39f34d6 into main Sep 11, 2026
2 checks passed
@mccanne
mccanne deleted the dot-fusion-none branch September 11, 2026 14:24
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.

2 participants