remote_name() for in_schema() etc

[mgirlich]

The documentation says

remote_name() gives the name remote table, or NULL if it's a query

But what exactly is meant by "name" when a table is specified via in_schema(), in_catalog() or DBI::Id()?

At the moment remote_name() returns these objects as they are. But from the name I'd rather expect to get a string.

I think:

• remote_name() should simply return the name of a table, e.g. for in_schema("my schema", "my table") it should return "my table". If the table was created via ident_q() or sql() it returns NULL
• we add another function remote_table() that returns a table identifier e.g. for in_schema("my schema", "my table") it should return in_schema("my schema", "my table"). If the table was created via ident_q() or sql() it should return them as they are.

[mgirlich]

@hadley What's your opinion here?

[hadley]

Hmmm, I don't really know what we or others use this function for so I don't have a strong sense of what the correct value should be. But one option you didn't mention is myschema.mytable etc.

[mgirlich]

We only really use it in a single place: in rows_*() to determine which table to modify.

With your proposal of myschema.mytable you meant the escaped version, e.g. `my_schema`.`my_table`, right? Returning an unescaped string feels like it could lead to subtle bugs.

A quick search on GitHub suggests that it isn't used very often and that people really expect a string to be returned. They use something like as.character(remote_name(x)) which will usually break their logic when applied on a schema.

So, I think it would be nicer if remote_name() returns the actual table name and we add remote_table() which returns a table identifier or the SQL code generated to create the table.

[hadley]

Yeah, that sounds fine. (And agreed about the escaping).