persistent_term: Introduce has_key/1 function

[Benjamin-Philip]

There is currently no way to quickly check if a key exists in persistent term. The closest you can get is the following function:

has_key(Key) ->
    try
        persistent_term:get(Key),
        true
    catch
        error:badarg ->
            false
    end.

This is clunky and not performant, especially given that is such a trivial function.

Originally suggested by @josevalim and approved by @jhogberg.

[mikpe]

What's the use-case? You can do this already using get/2 with a suitable choice for the Default value. And the result would be potentially invalid by the time you act upon it.

[Benjamin-Philip]

What's the use-case?
And the result would be potentially invalid by the time you act upon it.

Like José mentioned in the comment I linked, one use case is to check if a key exists before starting any expensive work to calculate the value for that key. Another use case could be assertions in tests (I replaced one such instance in #10535). Maybe it could also be used as a parameter in higher order functions.

In the case of acting solely on has_key returning false, you can use put_new as a fail-safe to stop different processes from stepping on top of each other. The validity of the final result that is put into the persistent term depends on your exact business logic.

If the work is expensive enough a check before starting may have positive performance benefits. Even considering the possible data race which falls through the check when two or more processes try to handle the same key at the same time.

In the case of putting a different value if a key exists, you should probably use get/2 if your new results depends on the previous value and erase otherwise.

For reference, maps and proplists already have equivalent functions (1, 2), so I'm sure this discussion has been had before. I agree with your concerns regarding data validity since persistent_term is a parallel and mutable construct, as opposed to maps and proplists which are sequential and immutable. Nevertheless, I still think this is a useful function as the equivalents in maps and proplists show.

You can do this already using get/2 with a suitable choice for the Default value.

I agree with you. My argument here is less about whether you can already get the same functionality with existing constructs, and more about the clarity and cleanness of your code. In my opinion,

case persistent_term:has_key(Key) of
    false -> ...
    true -> ...
end.

conveys the intention a lot more clearly and cleanly than

case persistent_term:get(Key, default_atom_that_probably_wont_be_used_anywhere_else) of
    default_atom_that_probably_wont_be_used_anywhere_else -> ...
    ValueThatWeProbablyDontWant -> ...
end. 

or

case catch(persistent_term:get(Key)) of
    {'EXIT',{badarg,_}} -> ...
    ValueThatWeProbablyDontWant -> ...
end. 

Of course in this case I've exaggerated the names I've used, but my point still stands. This gets further exacerbated when you use pass in a lambda with a case instead of just has_key to higher order function. I can think of a bunch of functions in Elixir's Enum where this is the case.

Specifically with using get/2, you have to handle the edge case of your default value actually being a valid pre-existing value. This effectively means we have to use get/1 with either catch/1 or a try-catch.

[mikpe]

Specifically with using get/2, you have to handle the edge case of your default value actually being a valid pre-existing value. This effectively means we have to use get/1 with either catch/1 or a try-catch.

Only if you try to make it generic. If you use get/2 for a has_key-like check in the code that would populate it, that code will usually know what the type should be and can select a suitable other type for the Default value.

For reference, maps and proplists already have equivalent functions (1, 2), so I'm sure this discussion has been had before.

Those are entirely different since they are immutable.

ets OTOH is concurrently mutable and does have ets:member/2 so there is precedent for this functionality.

[josevalim]

Only if you try to make it generic. If you use get/2 for a has_key-like check in the code that would populate it, that code will usually know what the type should be and can select a suitable other type for the Default value.

Honestly, having to compute a value that cannot exist in the stored value to use as a default is clearly a workaround the API. And in the cases where one does not exist, we are forced to namespace the value.

So I believe we either need a pesistent_key:find/1 (to mirror maps:find/2) or persistent_key:is_key/1 (to mirror maps:is_key/2). I don't think it should be called has_key/1 though, as far as I know that's not an Erlang idiom.

[Benjamin-Philip]

I don't think it should be called has_key/1 though, as far as I know that's not an Erlang idiom.

I can change it to is_key/1, I am not particular about the name.

So I believe we either need a pesistent_key:find/1 (to mirror maps:find/2) or persistent_key:is_key/1 (to mirror maps:is_key/2)

I think find and is_key, though similar solve two different problems. find (i.e. an atomic check exists and get) is useful if your new value depends on the old one. Whereas is_key (i.e. an atomic check exists) is better if it doesn't (or if you only proceed if no pre-existing value exist).

Due persistent_term's highly concurrent and mutable nature, you end up grouping these actions (put, get, erase, check exists and check doesn't exist) with each other for atomicity:

1. The recently added put_new/2 is effectively a check exists and put.
2. maps:find/2 is a check exists and get.
3. Python's pop is a get and erase.
4. put/2 is like an erase and put on a pre-existing key.
5. erase/1 as it is now (since it doesn't raise an error on invalid key) is a check and erase

Actually, these are all the valid the no repeating 2 action permutations, assuming put, get and erase raise on non-existent key:

1. get and put
2. get and erase
3. erase and put
4. check exists and put
5. check doesn't exist and put
6. check exists and get
7. check exists and erase

These are all the invalid (or useless) ones:

1. put and get (you already have the value, no point getting)
2. put and erase (just erase at one go)
3. put and check exists (no point checking if a key that you just put exists)
4. put and check doesn't exist (no point checking if a key that you just put doesn't exist)
5. get and check exists (no point checking if a key exists after getting it)
6. get and check doesn't exists (no point checking if a key doesn't exist after getting)
7. erase and get (will fail to get a key after erasing it)
8. erase and check exists (will fail since the key has just been erased)
9. erase and check doesn't exist (no point checking if a key doesn't exist after erasing)
10. check exists and check doesn't exist (mutually exclusive, will fail)
11. check doesn't exist and check exists (mutually exclusive, will fail)
12. check doesn't exist and get (will fail to get a non-exisistent key)
13. check doesn't exist and erase (will fail to erase a non-exisistent key)

$P(5, 2) = 20 = 7 + 13$, so that's all of them.

All 7 of these permutations, though similar to their parent constructs and each other, solve problems which cannot be solved by a sequential use of their parent or sister constructs, since you don't get atomicity (and thus correctness) by sequentially more than 1 function.

Of the valid permutations, we only support 3 in some way or another, so perhaps the following might also be in order?

1. swap/2 (get and put)
2. pop/1 (get and erase)
3. put_old/2 (check exists and put)
4. find/1 (check exists and get)

[josevalim]

persistent_term is used for concurrency but, because updates/deletions are expensive, you are usually not doing many of the operations above. In any case, I don't think that's the right way to go: we could easily think about a bunch different scenarios where we could use those, but if no one needs them, then it will just lead to API bloat. So I'd rather argue on a case by case basis.

That said, it makes sense that ETS has both member and lookup, because reading the value requires copying to the current process, so member can be considerably cheaper. I know there is no such copy for persistent_term but perhaps having the literal in the process even if it is not being used could be problematic if the term is then immediately removed from persistent term? But if there is no cost, is_key does not provide any benefit over find.

[Benjamin-Philip]

persistent_term is used for concurrency but, because updates/deletions are expensive, you are usually not doing many of the operations above. In any case, I don't think that's the right way to go: we could easily think about a bunch different scenarios where we could use those, but if no one needs them, then it will just lead to API bloat. So I'd rather argue on a case by case basis.

That makes sense. Though, if you continue the same process to 3 or more actions, you get that no more valid permutations exist. So, if it's just a matter of adding just 4 more functions to handle all possibilities, maybe it's worth the minimal bloat?

That said, it makes sense that ETS has both member and lookup, because reading the value requires copying to the current process, so member can be considerably cheaper. I know there is no such copy for persistent_term but perhaps having the literal in the process even if it is not being used could be problematic if the term is then immediately removed from persistent term? But if there is no cost, is_key does not provide any benefit over find.

I disagree. You can say the same of get. In fact it is equivalent to get(Key, error). I still think

case persistent_term:is_key(Key) of
    false -> ...
    true -> ...
end.

is clearer and conveys the intention better than

case persistent_term:find(Key) of
    error -> ...
    _KeyIDontWant -> ...
end.

perhaps having the literal in the process even if it is not being used could be problematic if the term is then immediately removed from persistent term?

Do you mean literals like binaries and bitstrings which may be sharing memory with terms in persistent term?