Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] Why can't spaces be used when checking if a redis key exists? #426

Open
rhalstea opened this issue Dec 8, 2020 · 3 comments
Open

Comments

@rhalstea
Copy link

rhalstea commented Dec 8, 2020

When checking if a redis entry exists the key isn't allowed to have any spaces:
https://github.com/Azure/sonic-swss-common/blob/master/common/dbconnector.cpp#L609-L613

However, that restriction doesn't seem to be enforced when creating (i.e. SET) or reading keys (i.e. KEYS). I was able to create an entry like:

VRF_TABLE:"vrf 80"

Is there a technical reason for this? I guess we can use KEYS to check for existence, but the O(1) time complexity of exists is much nicer.

@prsunny
Copy link
Contributor

prsunny commented Dec 9, 2020

I think even if you could create an entry with space in DB, the underlying infra may fail since some of these translates directly to create an entry in linux kernel where space is not allowed. So IMO, this is a general restriction.

@kcudnik
Copy link
Contributor

kcudnik commented Dec 9, 2020

in general is better to avoid spaces, in some places this can lead to regex wrong matching etc, in this case space it could be replaced by '=' or ':'

@rhalstea
Copy link
Author

rhalstea commented Dec 9, 2020

Granted VRF_TABLE was a contrived example and probably shouldn't have a space in the key. My question is more generic (e.g. maybe an ACL or some policy name). AFAICT today nothing is stopping a user from creating a key with a space:
https://github.com/Azure/sonic-swss-common/blob/master/common/dbconnector.cpp#L642-L647

The no space rule only seem to apply if the user wants to check if an entry already exists. Presumably the underlying infra would see an issue when the entry is created., Not when something checks if it already exists?

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

No branches or pull requests

3 participants