Add r.index_rename('old', 'new') command

[mlucy]

We need this command to make migration work smoothly for 1.14 . It should have the following behavior:

r.index_rename('old', 'new') # rename 'old' to 'new' unless 'new' exists
r.index_rename('old', 'new', overwrite: true) # rename 'old' to 'new', deleting 'new' if it exists

(See #2789.)

Are there any strong objections to this? (Consider it an accelerated RQL_proposal since we need it for release.)

[Tryneus]

Working on this now.

[Tryneus]

This is a bit rushed because of #2789, so any API discussion should happen quickly.

[timmaxw]

As part of the ReQL admin API we will be adding similar r.table_rename() and r.server_rename() API calls. We should presumably try to make the interface consistent. There would be no overwrite flag in the case of servers.

[Tryneus]

Proposed API

Parameters

This term is run on a table and takes exactly two arguments:

• Argument 0: TABLE
• Argument 1: STRING, old index name
• Argument 2: STRING, new index name

Optargs

• overwrite: BOOL (default=false), if true and an index with the new name already exists, it will be deleted

Return Value

• If successful, returns the object {renamed: 1}.
• If the old and new names are the same, returns the object {renamed: 0}.

Errors

• The new index already exists and overwrite is false - "Index name already exists on table db.table."
• The old index does not exist - "Index name does not exist on table db.table."
• The old or new name is the same as the primary index - "Index name conflict: name is the name of the primary key."

[Tryneus]

Forgot to specify the return value, added above.

[AtnNn]

It should also be an error if the first to arguments are equal.

[Tryneus]

@AtnNn, should that really be an error or just a no-op?

[AtnNn]

@Tryneus a no-op, returning {renamed: 0}

[Tryneus]

Can do, also adding errors for when the old or new name is the same as the primary key.

[danielmewes]

Looks fine. I'm not sure whether we really want to have the overwrite flag (and whether this is the right name, given that there will be a brief period during which the index will be unavailable), but I'm fine with it.

We should make sure that this works correctly with backfilling.
I believe if we don't do anything special what will happen is that if you backfill from the primary to a secondary that has missed an index_rename() the secondary will drop the current index and recreate a new one under the new name.
That could be ok, but it is a bit problematic because rebuilding the index like that will also affect other shards that are on the same machines.
The logic for this is in rdb_receive_backfill_visitor_t::operator(const backfill_chunk_t::sindexes_t &) in store.cc and store_t::set_sindexes(...) in btree_store.cc.

This is also very remotely related to #2477, in the sense that if we wanted to rewrite that code (which we probably shouldn't at this point) we would probably want to fix both.

[Tryneus]

This is implemented and up in review 1863.

[Tryneus]

This has been approved and merged to next in commits:

• 44c576d - server support
• d459e8a - driver support
• 98f5bdb - polyglot tests
• 1b3497d - unit tests

Will be in release 1.14.

[mlucy]

<.< Curse you GitHub for including labels in the history.