feat: add format ReQL command

[gabor-boros]

• I have read and agreed to the RethinkDB Contributor License Agreement http://rethinkdb.com/community/cla/

Description

This PR resolves #3353 by adding the mentioned r.format command. The command formats the template string, passed as the first parameter, using the object provided as the second parameter.

Related PRs

• docs: update reql_docs.js and add format #7065
• docs: add format ReQL command docs#1332

Limitations

• The object must have string keys and string values.

Questions

• Should we support int and double in the object? If yes, what would be the expected way of lexical casting of doubles to strings withouth adding extra trailing zeros or truncating the doubles?

Leftovers

• Cut a new 2.4.x branch before merging and rename the current one to main

The drivers shouldn't release format support until 2.5.0.

• feat: reql format command rethinkdb-javascript#14
• feat: update protobuf to allow reql format rethinkdb-ruby#28
• Moving from Python 2 to Python 3 and bump to v2.5 rethinkdb-python#277
• Implement format in the Java driver (cc: @adriantodt)

Other (maintained) drivers:

• Implement format in the Go driver (cc: @CMogilko)
• Implement format in the Typescript driver (cc: @atassis)

[srh]

Is it going to be r.fmt or r.format?

I would expect it to support non-string values and use r.coerceTo('string').

[srh]

There are some bugs, and I think the treatment of format strings with unmatched curly braces is bad.

[srh]

Is it going to be r.fmt or r.format?

You wrote format or FORMAT the whole time in the code, which is why I'm asking. I think r.format would be fine.

[gabor-boros]

Thank you for the review, @srh . I'll go through the suggestions in the upcoming days. Also, I'll use the r.format version instead of r.fmt. That seems a bit more correct to me.

[gabor-boros]

@srh except coercing all comments are addressed. The error message wording is probably not the best, but I'm opened for any suggestions.

Is it going to be r.fmt or r.format?

Although the original proposal was fmt, I think format would be a bit better.

Regarding the coercing

As I figured out, we could probably use the minidriver to evaluate the following, but I'm struggling how to properly do that. Do you have any suggestions? Nevermind. I figured it out.

datum_t field = datum.get_field(datum_string_t{param_name});
r.expr(field).coerce_to("STRING")

Update: I'm going to update the PR tomorrow with the latest changes and adjusted polyglot tests

[gabor-boros]

Now, this is ready for review.
Update: ReQL tests are obviously failing as they have no driver implementation yet.

[srh]

We might also want to reserve a character for use in format strings for use in specifying how the data is formatted. C# uses , and :, while Rust uses :. It might be that : is used in some field names. Maybe there is a better character to use.

We might also want to reserve . to allow field.subfield syntax in the future.

[srh]

Const reference nitpicking.

[srh]

In terms of keeping characters for formatting, I would personally prefer the colon approach or percent sign. These two characters are rarely used in document keys -- I hope -- and both are used by other languages like Python (%), Rust (:), Java (%), etc.

What do you think nested fields might look like?

Maybe we should reserve an escape or quoting character that could be used for field names.

[gabor-boros]

@srh

What do you think nested fields might look like?
Maybe we should reserve an escape or quoting character that could be used for field names.

I had some time today to think about it, though I couldn't come up with a better idea. I think there are two questions:

• What character(s) do we want to reserve for later use for formatting?
• Do we want to support nested field access? If yes, how would we access the nested field?

"What character(s) do we want to reserve for later use for formatting?"

The original proposal was to have a string replacement like this:

// Replace the placeholder with string representation
r.format("Simple formatting: {x}", {"x": "})

To define formatting, we could use the %, :, or , sign, but using that could make the placeholder a bit unreadable:

// Format the replacement as a floating point number v1
r.format("Temperature: {temp%f.2}", {temp: 32.123})

// Format the replacement as a floating point number v2
r.format("Temperature: {temp:f.2}", {temp: 32.123})

// Format the replacement as a floating point number v3
r.format("Temperature: {temp,f.2}", {temp: 32.123})

Out of the three the third variant, using ,, maybe the most readable. Although I personally prefer % it is more important to keep the placeholder readable, so I'd say to reserve the , character.

"Do we want to support nested field access? If yes, how would we access the nested field?"

If I understand correctly, this would be the usage example (assuming the subfield access character is .):

// Replace the placeholder with string representation
r.format("Simple formatting: {x.y}", {x: {y: 1}})

Well, I would not support this. It could bring too many edge cases to the table. Consider the following example:

r.format("Simple formatting: {x.y}", {"x.y": 1})

In this case, the key has the access character (.) in its name, furthermore, the value is not an object, but a number. Or even worse:

r.format("Simple formatting: {x.y}", {"x.y": {y: 1}})

What would we render here? The key is x.y, but the value of x.y has a key y as well.

I would simply say that nested field's shouldn't be allowed as of now. If someone brings a proposal for solving all issues mentioned above, we could consider that, but I don't have a good proposal for solving the issue.

---

I went ahead and reserved the , character to reduce the number of iterations, though I'm opened for suggestions.

[gabor-boros]

FTR: Before we merge the Python driver must be adjusted too.

[gabor-boros]

The PR is rebased; I'll finish the PR after the next 2.4.x release. Basically, the test-runner and related python code must be bumped to work nicely with Python3 and we are good to go.

[gabor-boros]

@srh If I would want to fix the Python tests to work with the new driver implementation and Python 3, I would at least 4x the PRs size with somewhat irrelevant changes. How about the following?

• I move the second commit (6746820) to a separate PR where I update the repo to use Python 3
• I create a new default branch from the current v2.4.x and call it main
• Then I merge this PR into main which will not affect the new 2.4.x release.

We already discussed the second one, but wanted to bring into your attention again as I've seen your plans to release a new 2.4.x version.

[srh]

@gabor-boros I am hoping to get FreeBSD-related compilation fixes merged before forking the branches.

[gabor-boros]

@srh 👌I'll wait for that to not interfere

[srh]

Just reminding myself: after we break off the Python/test code changes and merge these, we will want to

• make driver changes
• update the JS driver in the admin UI in old_admin in a later PR