Concat with nil values
The built in concat calculation returns nil when any of the values it is concating is nil. I need a nil friendly concat and I could use something like this (which only works on the data layer and is postgres specific):
I'd prefer to write a custom calculation that I can reuse (so the calculation can be run in Ash land after the record is retrieved from the db), but if I create a custom calculation (to use
concat_ws) it is still only going to be suitable only for postgres datalayer. Is there a way that ash_postgres could use concat instead of || as an option?
I'm probably overthinking it and should just use the above calc and move on!36 Replies
Hm…I don’t think we can do it now unless we did a major release. But also the elixirland version of
:<> does the same thing
(Because we emulate nil bubbling up like NULL)yep, it's consistent, which is cool
But you could always do
(foo || “”) <> (bar || “”)Wouldn't that duplicate the joiner for any empty values?
yeah I'd be left with a dangling separator
… ohhh I see what you mean.
I mean, we can just add
concat_ws or similar expression in ashconcat_ws basically behaves like Enum.join/2Perhaps we should add
join/2 to the expression syntax?
Since it’s useful in general.I think that would really be the ideal solution, yeah.
Although join is maybe a confusing word in that context lol
Honestly,
concat_ws is kind of confusingly labelled.how would that map to ash_postgres? it would be a postgres only calc?
Because of db joins
Ahh, yeah... lol, either way you get term overloading.
Nah, it would use concat_ws in postgres and then equivalent elixir code in elixirland
yep, and ets?
Ets just uses the elixirland logic
But yeah essentially.
Honestly I’m amazed there hasn’t been more of a push to build out the “expression standard library”
I thought people would be champing at the bit
Ok sweet. Is it asking for future issues if ash (core) contains calcs that are postgres specific? Or is that future Ash's problem?!
Just thinking if more data layers come along
Well, that’s not really how it works. The calc exists in core, but the implementation exists in ash_postgres.
ah ok
And it’s fine for there to be expressions not supported by some data layers
But anything we could reasonably expect most data layers to support can go in ash core
Makes sense, you're talking about expressions, I was still thinking about a custom calculation... I'll have a look at expressions and see what I come up with
Ah, yeah I see what you mean
Ok, I think I'm getting it (adding expressions). What's the name/api for this concat function? I'm going with
expr(string_join(" ", [first_name, last_name])), where the values are passed in a list. I liked the look of how fragment handles many unknown args (:vararg) but I'm guessing that should be avoided and is used with fragment because that's how it's done in ecto?Correct, nothing but fragment should use vararg TBH
not when we can do
string_join([...])cool. so i left the separator as the first arg. I was thinking about having 2 signatures, one with a separator and one without
I even considered not doing it for
fragment and making fragments in ash be fragment("foo", [foo, bar])
but its too late now, that ship has sailed
yeah, that sounds like a good idea.happy with
string_join for now? I want to avoid concat as that is a built in calculation... and join as said could be confusingYeah, I think
string_join is great.So I chucked together a couple of PRs and thought everything was cool (the expression works great in the data layer), but it doesn't work in the elixirland. I think i'm misunderstanding how to interpret the args in evaluate
It's tricky because when I inspect the values coming in I don't really get any info (maybe no inspect protocol implemented on the arg?)
But join fails with
** (Protocol.UndefinedError) protocol String.Chars not implemented for address_1 of type Ash.Query.Ref (a struct)
https://github.com/ash-project/ash/blob/ad8b892ba0266f9b004eac8b30c166b052c50920/lib/ash/query/function/string_join.ex#L15That sounds like a bug somewhere in our evaluation logic
we shouldn't pass things like that into the evaluate function
maybe because of the array breaking some value checking. I'll take a look. The PRs look great, been busy today
Ok no worries, or rush.
I just added a failing test to the pr
Fixed the issue, released ash, releasing ash_postgres now. Great work 😄
We can potentially change the way the default concat calculation works (or just remove it) in the next major release
Awesome, yeah I was wondering what to do about the concat calc. I guess we'll still need implement something that handles nil's like concat (ie returns nil if any of the elements are nil). I was thinking about tweaking the existing
concat calculation to take a skip_nils? option so you can go either way, but I think we'd have to duplicate the elixirland logic? Is there a way to let a calculation just defer to an expression (ie not provide a calculate callback) and let the correct expression get used (elixirland or data layer)?Yeah, just has to no define the
calculate/3 callbackok sweet, I'll play with that