Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

ksLookup options #4222

Closed
kodebach opened this issue Jan 6, 2022 · 4 comments
Closed

ksLookup options #4222

kodebach opened this issue Jan 6, 2022 · 4 comments

Comments

@kodebach
Copy link
Member

kodebach commented Jan 6, 2022

Related to #3979 and possibly as another part of #4201, we should think about which options of ksLookup are actually still relevant.

The available options are as follows:

Note: Not all of these options are declared in kdb.h as elektraLookupFlags, but ksLookup doesn't do any checks or masking, so all of them can be used.

  • KDB_O_DEL: not necessary, but useful option, if the key isn't needed afterwards, but ksLookupByName wasn't an option
  • KDB_O_CREATE: not necessary, but useful option, if the key is needed afterwards, but might not exist yet
  • KDB_O_SPEC: not really sure why this exists or what purpose it serves
  • KDB_O_NOSPEC: AFAICT only used internally to avoid recursion; shouldn't be supported by public API
  • KDB_O_NOCASCADING: might make sense for testing and internal use, but shouldn't be supported by public API
  • KDB_O_NODEFAULT: might make sense for testing and internal use, but shouldn't be supported by public API
  • KDB_O_CALLBACK: AFAICT only used for the meta:/callback metadata; shouldn't be supported by public API
  • KDB_O_OPMPHM: might make sense for testing and internal use, but shouldn't be supported by public API
  • KDB_O_BINSEARCH: might make sense for testing and internal use, but shouldn't be supported by public API

So this leaves only KDB_O_DEL and KDB_O_CREATE for the public API. They are surely nice to have, but at this point the question is: Do we need them? Is this minimal?


Two tangentially related questions that might need their own issue:

  • Should ksLookupByName be core API? It's not really minimal, so it might be better as a part of the proposed libelektra-operations?
  • Should elektraLookupBySpec be part of the core, or should we use meta:/callback to get a callback into the spec plugin?
    The plugin can then process meta:/fallback etc. and return the correct key. I think with such a callback we could also handle # and _ wildcards much better. In fact, spec might (optionally) be able to do all processing lazily upon lookup.
    The main benefit of this change would be that alternative implementations (e.g. Rust) don't need to duplicate this lookup procedure. They just need to know about the cascading order and call any meta:/callbacks.
@markus2330
Copy link
Contributor

Do we need them? Is this minimal?

No, you are right, we can remove that flag in the public API if we have a proper replacement for KDB_O_POP.

Should ksLookupByName be core API?

I agree.

It's not really minimal, so it might be better as a part of the proposed libelektra-operations?

As also ksCut and many others could have a ByName variant, we need to watch out that libelektra-operations doesn't get too fat.

@kodebach
Copy link
Member Author

No, you are right, we can remove that flag in the public API if we have a proper replacement for KDB_O_POP.

Which flags can be removed? All of them? I didn't even mention KDB_O_POP (it will be removed anyway).

Should ksLookupByName be core API?

I agree.

You agree, it should be core API or you agree it should be removed?

As also ksCut and many others could have a ByName variant, we need to watch out that libelektra-operations doesn't get too fat.

  1. I thought the purpose of libelektra-operations is to be "somewhat fat" to make using Elektra easier.
  2. We could always split libelektra-operations into smaller more focused libraries (I don't like the name anyway). But too many shared libraries is also not good.
  3. A *ByName variant only makes sense, if there is any benefit to the stack allocated Key. With ksLookup this makes the most sense, because it theory it should be O(1) and not result in any new memory allocations. For ksCut it makes less sense, ksCut already allocates new memory and moves stuff around. Also ksLookup might a called many times in a row, while ksCut normally is just called once.

@markus2330
Copy link
Contributor

You agree, it should be core API or you agree it should be removed?

With your sentence "It's not really minimal, so it might be better as a part of the proposed libelektra-operations?" i.e. I agree moving it to libelektra-operations.

I thought the purpose of libelektra-operations is to be "somewhat fat" to make using Elektra easier.
We could always split libelektra-operations into smaller more focused libraries (I don't like the name anyway). But too many shared libraries is also not good.

Yes, a good architecture for these supporting libraries is still missing.

@kodebach
Copy link
Member Author

Yes, a good architecture for these supporting libraries is still missing.

See #4245 for a suggestion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants