We missed this as part of 7.0, but we should probably change the default in 8.0 so that scripts are only able to access keys they declared to access. This better standardizes the expected behavior of scripts, and adds a clearer pathway for deprecating it in the future if we want.
Suggested implementation:
1. Add a new field for lua scripts which indicates the API version. This will be bumped to 2 with the shebangs and will be an optional field for function registration.
2. By default, a dictionary will be maintained of each key that was pre-declared. If a key is accessed that does not exist within that database, an error will be thrown.
3. Add a new flag, no-required-key-declaration, which allows scripts to be compatible with the previous versions and bypasses this check.
Some notes: 1. This means that scripts can not access multiple databases, since they will only be able to declare the keys being accessed in the current database. 2. We might consider implementing this in such a way that it could then support background commands, since we can reason about which keys are being accessed.
Comment From: oranagra
this is somewhat related to #10951, and while i agree we should add some feature like that, we know that for some scripts (or even common use case of functions) it's not an option. i'm not sure we can afford to make it the default. although being optional i guess no one will ever bother to set it, so maybe making it the default will force anyone who relies on extra keys to add the flag. still, i'm not sure it'll make sense to bump a version just for that (with functions and shebangs it was easier... wish we would have discussed this sooner).
I think we should keep discussing it now and come up with a plan ASAP, maybe add an optional flag in 7.2 so we can change the default in 8.0.
@redis/core-team @meir WDYT?
Comment From: madolson
We said we want to do this as an opt-in way starting in Redis 8.
Comment From: yossigo
Add a new flag, no-required-key-declaration, which allows scripts to be compatible with the previous versions and bypasses this check.
If we have a version flag, this could be redundant as scripts can always not opt-in V2. It will also help drive key declaration adoption since users with other compelling reasons to upgrade their version flag will be forced to fix the scripts.
We have to continue validating this against various scripting use cases, as I think we may be overlooking some cases where declaring keys is not possible or practical.
Comment From: mgravell
This means that scripts can not access multiple databases, since they will only be able to declare the keys being accessed in the current database.
If this remains the case, consider whether select should itself be blocked; right now, my library has to consider the database "unknown" after a script, and issue a select for reliability afterwards; if select literally wouldn't work, then I can skip this
Comment From: yossigo
@mgravell I don't think this is necessray, select in a script should only affect the state of the script's client and leave no effect on the real client when the script terminates.