valkey-io / valkey-doc

Other
17 stars 25 forks source link

`/topics/lua-api.md` has several problems, RESP naming, and other issues. #116

Open stockholmux opened 1 month ago

stockholmux commented 1 month ago

In pre-publishing review #91, I discovered a number of issues with lua-api.md

FYI: A general problems exist in this document with hash links.

This is a helper function that returns an error reply.

Error reply links to a non-existent hash link /docs/topics/protocol#resp-errors

This is a helper function that returns a simple string reply

simple string reply links to a non-existent hash link /docs/topics/protocol#resp-simple-strings

This function allows the executing script to switch between Valkey Serialization Protocol (RESP) versions

'Valkey Serialization Protocol' is not a thing, should still be Redis Serialization Protocol.

As of Redis OSS version 2.6.0, scripts were replicated verbatim, meaning that the scripts' source code was sent for execution by replicas and stored in the AOF. An alternative replication mode added in version 3.2.0 allows replicating only the scripts' effects. As of Redis OSS version 7.0, script replication is no longer supported, and the only replication mode available is script effects replication.

This should be refactored as 2.6 replication is irrelevant now, and maybe just include 3.2.0 as "before 7". Maybe we don't need to document server.set_repl(x) at all? I'm unclear if it's relevant since there is only effect replication now.

Valkey' replies and Lua's data types and a one-to-one mapping between Lua's data types and the Valkey Protocol data types.

'Valkey Protocol' should be replaced by RESP

* [RESP2 integer reply](protocol.md#resp-integers) -> Lua number * [RESP2 bulk string reply](protocol.md#resp-bulk-strings) -> Lua string * [RESP2 array reply](protocol.md#resp-arrays) -> Lua table (may have other Valkey data types nested) * [RESP2 status reply](protocol.md#resp-simple-strings) -> Lua table with a single _ok_ field containing the status string * [RESP2 error reply](protocol.md#resp-errors) -> Lua table with a single _err_ field containing the error string

None of these hash links exist on protocol.md

* Lua number -> [RESP2 integer reply](protocol.md#resp-integers) (the number is converted into an integer) * Lua string -> [RESP bulk string reply](protocol.md#resp-bulk-strings) * Lua table (indexed, non-associative array) -> [RESP2 array reply](protocol.md#resp-arrays) (truncated at the first Luanilvalue encountered in the table, if any) * Lua table with a single _ok_ field -> [RESP2 status reply](protocol.md#resp-simple-strings) * Lua table with a single _err_ field -> [RESP2 error reply](protocol.md#resp-errors)

None of these hash links exist on protocol.md

* [RESP3 null](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#null-reply) -> Luanil.

  • [RESP3 true reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#boolean-reply) -> Lua true boolean value.
  • [RESP3 false reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#boolean-reply) -> Lua false boolean value.
  • [RESP3 double reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#double-type) -> Lua table with a single _double_ field containing a Lua number representing the double value.
  • [RESP3 big number reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#big-number-type) -> Lua table with a single _big_number_ field containing a Lua string representing the big number value.
  • [Valkey verbatim string reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#verbatim-string-type) -> Lua table with a single _verbatim_string_ field containing a Lua table with two fields, _string_ and _format_, representing the verbatim string and its format, respectively.

None of these hash links exist on https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md

You can call the SELECT command from your Lua scripts, like you can with any normal client connection. However, one subtle aspect of the behavior changed between Redis OSS versions 2.8.11 and 2.8.12. Prior to Redis OSS version 2.8.12, the database selected by the Lua script was set as the current database for the client connection that had called it. As of Redis OSS version 2.8.12...

Refactor this passage to just put the 2.8.12 way as the way. The rest is irrelevant to Valkey.

madolson commented 1 month ago

Some of these have been resolved in https://github.com/valkey-io/valkey-doc/pull/85. I need to update that PR.

zuiderkwast commented 4 days ago

The links to protocol.md#xxxxx are fixed in #153.

zuiderkwast commented 2 days ago

I'm deleting all references to older versions like "Since 2.6.0", but to help users compare functionality when they upgrade, we've said that we'll keep documentation for the still maintained versions, i.e. 6.2+.

I'm removing most of the text about verbatim replication, but keeping small parts, because 6.2 is still supported.

Changing "Valkey protocol" to "RESP protocol". (RESP is not an acronym for anything. It's just a name.)

Moving the description of the server object to just above where the fields are documented.

Some formatting and minor changes.