SUBSTR was renamed, but currently works. Shouldn't we have a SUBSTR doc?

[chayim]

Calling SUBSTR against even the most recent Redis instances works, though the command has been renamed. Shouldn't we at least have redis.io/subst redirect to the appropriate command?

[yoav-steinberg]

This is mentioned in the GETRANGE documentation. I think since this was so long ago and isn't even documented for over 10 years we might prefer leaving it undocumented and perhaps even chucking SUBSTR completely in a future release. @itamarhaber ?

[itamarhaber]

Agreeing w/ @yoav-steinberg on this. In v7, this will/should have the "internal" flag set to true to mask it from users.

[yoav-steinberg]

@itamarhaber I'm not exactly sure what you mean by "internal" flag but can you open a ticket/PR for this in the redis repo. I'll go ahead and close this one. @chayim Thanks for pointing this out, I guess we'll follow up in the main redis repo.

[oranagra]

@itamarhaber @yoav-steinberg i don't know if we can / should chuck SUBSTR (could break someone's app without a real good reason). i agree it's odd that it's in the redis command table and COMMAND command response and not documented. i suggest adding a short redirect page for it in commands.json (which is what's gonna happen anyway soon when redis is the commands.json SSOT)

[itamarhaber]

My 5 agorot:

1. I believe SUBSTR has been in Redis since v1.
2. We can't remove it entirely due to backcompat, so we should consider it as deprecated as of 2.0.0" and replaced by GETRANGE.
3. The new Redis command table has a new proposed flag called "internal" for internal commands, such as REPLCONF and XSETID. Obviously, this command doesn't fall into that category, so please ignore my above comment.
4. A new flag in the table, still under debate, is the "undocumented" flag, which is basically for this one but also others (potentially internal) that do not have a documentation page. Being the one who proposed that flag, I think we don't really need it.
5. All that being said, I think this can be resolved by creating a commands/substr.md file with the command's description and the deprecation notice.

[itamarhaber]

In future versions of the docs it can look something like this

[yoav-steinberg]

My 1 drachma: I think, given this was deprecated years ago we can chuck it and advertise this as a breaking change in v7.0. I know it might break apps, but they can decide to either fix their code or stay with and old version. This is in line of my general attitude to backwards compatibility issues, which I know not everyone agrees with which is fine. So if we're not going to chuck it, then I'm fine with @itamarhaber's suggestion above, and happy with someone doing a PR for this.

[chayim]

My 1 dirham: I'm pro breaking things - if it's because we're adding or changing functionality. At that point it's adding (possible) value for breaking things. But removing a command, I personally think is the bad sort of breaking change. By all means mark it as deprecated along the way though.

[oranagra]

i'd argue that even if we chuck it, redis.io should still document it, since it is being used as the documentation for old versions too.

[mich-elle-luna]

this is documented now https://redis.io/commands/substr/ as deprecated