Libelektra: keyRel

[markus2330]

We currently have two different keyRel implementations:

• keyRel (in core)
• KeyRel2 (in proposal)

We should either:

• remove keyRel2
• rename keyRel2 to keyRel
• remove keyRel and keyRel2

@PhilippGackstatter How much effort would it be to replace all keyRel with keyRel2 calls?

In the current form, keyRel(2) can be in libease, as it only calls other methods.

Question to everyone:

• Which API do you like more?
• Is keyRel needed at all?

[kodebach]

As far as I can see, keyRel isn't even fully implemented.

IMO keyRel has the better API, but keyRel2 has additional functionality that may be useful. The return codes should all be positive, except -1 for null pointers, and indicate the relation between the two keys. The third argument (KeyRelType in keyRel2) should only be an option argument, setting e.g. whether the namespace shall be ignored and how cascading keys are treated.

It should also be possible to check all of the various relations using only the unescaped name, so the method could easily be in elektra-ease.

[markus2330]

As far as I can see, keyRel isn't even fully implemented.

Yes.

IMO keyRel has the better API

The problem of the keyRel API is that there are many ambiguous cases: sometimes more than one relation is true (e.g. same hierarchy, below and direct below can be true at the same time). Of course we could improve the implementation and make unambiguous return codes by giving every position within the tree a clear return code but then the caller might not get the answer he/she wanted. In keyRel2 you get exactly the answer you asked for.

It should also be possible to check all of the various relations using only the unescaped name, so the method could easily be in elektra-ease.

If we need it, I agree. But do we need it at all?

[kodebach]

that there are many ambiguous cases

I first assumed, we would return the closest relation. So for user/key and user/key/a we would return "direct below", which implies "below" and "same namespace".

But we could also return a bitfield, that indicates all relations. Then the user can check for the ones they are interested in. This also allows to check multiple relations at the same time and still allows to differentiate between them. I wouldn't have to call the method multiple times e.g. if I want to do A if we have a direct below relation, B if we have another below relation and nothing otherwise.

If we need it, I agree. But do we need it at all?

I don't know, whether we really need any form of keyRel. Where is it currently used?

The switch to using the unescaped name also was just a suggestion, in case one of the methods used in keyRel is not intended for use outside of the core.

[markus2330]

I first assumed, we would return the closest relation. So for user/key and user/key/a we would return "direct below", which implies "below" and "same namespace".

Yes, in the current version this works. But it stops working when adding further relations.

But we could also return a bitfield, that indicates all relations.

Then we might need to calculate many relations the user actually is not interested in.

Where is it currently used?

For keyRel2 see #2993. @PhilippGackstatter can you check for keyRel? (If the occurrences are easily replaceable with keyIsBelow or similar)

[PhilippGackstatter]

@PhilippGackstatter can you check for keyRel? (If the occurrences are easily replaceable with keyIsBelow or similar)

From briefly scanning the references, most occurences of keyRel either require only a replacement with one of the keytest functions, some two of them, but nothing more complex.

@PhilippGackstatter How much effort would it be to replace all keyRel with keyRel2 calls?

It's not heavily used, so it shouldn't be too much effort.

I don't know, whether we really need any form of keyRel. Where is it currently used?

As stated above, in most cases it can be replaced by just one other function, so I also don't think it's needed.

[markus2330]

Ok, then let us get rid of keyRel and keyRel2.

[PhilippGackstatter]

With keyRel2 gone in #3038, the helper functions keyAsCascading and keyGetLevelsBelow aren't used anywhere. Is this useful functionality that we should keep or should we get rid of them?
Since they are part of kdbproposal.h we need to at least move them.

[markus2330]

We should get rid of them. Reintroducing is easy, removing is not possible after 1.0.0.