search

ash-project / ash

A declarative, extensible framework for building Elixir applications.
https://www.ash-hq.org
MIT License
1.62k stars 218 forks source link

Add `%Ash.Expr{}`, which will be returned from `expr/1` #1069

Open zachdaniel opened 6 months ago

zachdaniel commented 6 months ago

Its not possible to easily tell “did they pass an expression”. You can use Ash.Expr.expr? but that literally traverses the entire value.

I think we can actually change this transparently to end users, since they shouldn’t be manipulating expressions, but we’ll need to adjust a bunch of code to just pass the contained expression by. This will also be useful in disambiguating interfaces.

ghalestrilo commented 2 months ago

Hi @zachdaniel . Can I help with this one? If so, please share some pointers to help get started.

zachdaniel commented 2 months ago

That would be great :)

The first thing to be done would be to make the expr/1 return %Ash.Expr{expr: <the_expr>}. Then there will be a whole bunch of places that match on expressions that have to unwrap and/or work for that. To see what those places are, we'd likely have to search for expressions like %Ash.Filter{ and %Ash.Query.BooleanExpression{. Those would need to either update/unwrap the new struct (depending on what they do).

FWIW it's going to be a pretty significant change. But end users won't have to know about it.

ghalestrilo commented 2 months ago

Awesome, thanks! I'll give it a go and report back

ghalestrilo commented 2 months ago

One thing I noticed: we didn't describe where (at which level of abstraction) this struct should be visible. AFAICT load uses query which in turn uses calculation which finally uses expr - maybe wrapped in something like BooleanExpression. Is this boundary defined?

edit: expand on "where"

zachdaniel commented 2 months ago

It should be invisible when inspecting 👍