possible to type convert/assert to a map?

[Dan Kortschak]

We have just had a report of an issue in one of our expressions after
bumping from v0.17 to v0.18.

The expression makes use of an extension, `drop` that removes paths
from objects, it is an instance method on list<dyn> and map<K,V>. Due
to improvements in type checking in v0.18 the expression fails with "no
overload" when the instance is indexed from a container since it comes
out as a dyn (It's not entirely clear to me why this worked in v0.17,
but that's a different issue).

Minimal repro:

data:
```
{"a":{"b":1,"c":2},"d":{}}
```
expression (data is object above):
```
data["a"].drop("c")
```
The expected evaluation (and what v0.17 gives):
```
{"b":1}
```

The failure can be worked around by making use of the list<dyn>
signature
```
[data["a"]].drop("c")[0]
```
which recovers the desired behaviour, but is pretty awful.

The extension could be changed to include an instance method on dyn,
but I don't know that the dispatch would choose the least dynamic
option first (which would be desirable).

So, is there a way to assert/hint that a value is a particular type,
failing with an error if not? We know that the value of `data["a"]` is
a map, but I cannot see a way to let the type checker know this.

thanks
Dan

[Tristan Swadell]

Hi Dan,

Could you point me to the environment setup? My expectation would be that the `dyn` would permit the type-checking to complete.

-Tristan

--
You received this message because you are subscribed to the Google Groups "CEL Go Discussion Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cel-go-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/cel-go-discuss/a19fb49bd592b4470c093f2e5ac8fbe2168c6bf0.camel%40kortschak.io.

[Tristan Swadell]

Here's roughly what I tried doing, and it compiled okay. Perhaps this is a runtime issue you've encountered?

kvMap := MapType(TypeParamType("K"), TypeParamType("V"))

eList := ListType(TypeParamType("E"))

env := testEnv(

t,

Variable("data", MapType(StringType, DynType)),

Function("drop",

MemberOverload("drop_list_map", []*Type{eList, StringType}, eList),

MemberOverload("drop_map_key", []*Type{kvMap, StringType}, kvMap)))

_, iss := env.Compile("data['a'].drop('c')")

if iss.Err() != nil {

t.Fatalf("env.Compile() failed: %v", iss.Err())

}

[Dan Kortschak]

On Mon, 2024-04-08 at 15:51 -0700, 'Tristan Swadell' via CEL Go

Discussion Forum wrote:
> Could you point me to the environment setup? My expectation would be
> that the `dyn` would permit the type-checking to complete.

The type checking is doing the right thing here.

This is the extension's signature definition:
https://github.com/elastic/mito/blob/typechecking/lib/collections.go#L300-L321

Here is a change that demonstrates the behaviour:
https://github.com/elastic/mito/commit/cd864c66b2dd3cbe71c368333709cf90c50623fa

The want.txt in each of the added test cases shows the behaviour.

If the `state["a"]` eval is wrapped in `dyn()` we get the expected "no
such overload: drop" since the instance method `drop` is on mapKV and
list<dyn>.

The issue is that the data is coming in as JSON, so there is
essentially no type information by the time the CEL code sees it.

The way out of this would be to not insist on mapKV and just work on
dyn, but I'd like to know if there is an alternative first.

thanks
Dan

[Tristan Swadell]

Hi Dan,

The one comment I have is that the functions are taking type params, but not marked as taking type parameters within the collections.go file. I've tried reproducing the issue with a subset of the declarations and have not been able to at compile time for v0.18.0+, but I would try switching to a `NewParameterizedInstanceOverload` declaration, or convert to the new style of function declarations which detects type params automatically (making function signature declaration a bit less of a foot-gun).

-Tristan

--
You received this message because you are subscribed to the Google Groups "CEL Go Discussion Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cel-go-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/cel-go-discuss/0774ab62c87e16a883d3d8b960d80714ff85b673.camel%40kortschak.io.

[Dan Kortschak]

On Mon, 2024-04-08 at 21:37 -0700, 'Tristan Swadell' via CEL Go

Discussion Forum wrote:
> The one comment I have is that the functions are taking type params,
> but not marked as taking type parameters within the collections.go
> file. I've tried reproducing the issue with a subset of the
> declarations and have not been able to at compile time for v0.18.0+,
> but I would try switching to a `NewParameterizedInstanceOverload`
> declaration, or convert to the new style of function declarations
> which detects type params automatically (making function signature
> declaration a bit less of a foot-gun).

Thanks, yeah. That has been on the plans, but as I'm sure you feel,
refactoring often takes a back seat (all my new code uses the new
declarations).

I'll take a look at doing the conversion to the new API and see if that
fixes the issue for us.

thanks
Dan

[Tristan Swadell]

Hi Dan,

You could start by just converting that one function. I've tried both ways (old and new) and haven't been able to repro the type-check error yet. If I have more time, I'll clone the repo and try the tests locally.

-Tristan

--
You received this message because you are subscribed to the Google Groups "CEL Go Discussion Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cel-go-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/cel-go-discuss/64615c7b1f4114b946e8e0e258681317ac1c5a9b.camel%40kortschak.io.

[Dan Kortschak]

On Tue, 2024-04-09 at 08:35 -0700, 'Tristan Swadell' via CEL Go

Discussion Forum wrote:
> You could start by just converting that one function. I've tried both
> ways (old and new) and haven't been able to repro the type-check
> error yet. If I have more time, I'll clone the repo and try the tests
> locally.

Thanks, Tristan.

I've confirmed that the new function construction resolves the issue.

Dan

[Tristan Swadell]

I'm a bit surprised that I wasn't able to repro it with the old method, but I'm glad the update fixed the issue!

-Tristan

--
You received this message because you are subscribed to the Google Groups "CEL Go Discussion Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cel-go-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/cel-go-discuss/0d161ddbb8dd3211c060396bdbadfe745ee0f9f7.camel%40kortschak.io.