Page MenuHomePhabricator

Support "%R" (Database + Table Ref) in qsprintf(...)

Authored by epriestley on Oct 26 2018, 3:09 PM.



Ref T13210. Ref T11908. In the future, we'd like to be able to use one port-to-port database connection to issue queries for multiple databases (e.g., both Differential and Maniphest) if both applications are on the same host.

The big issue with this is that we currently run queries like SELECT * FROM xyz, and assume the database name based on the connection state. Instead, we want to run SELECT * FROM, which will work no matter which database the connection is currently USE ..-ing.

To support this, add a %R conversion to qsprintf(...) which takes a "database + table ref" object. This is a new type of object which stores both a database name and a table name, so we can build "".

Test Plan

See the next change for callsites and unit tests.

Diff Detail

rPHU libphutil
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Oct 26 2018, 3:09 PM
epriestley requested review of this revision.Oct 26 2018, 3:09 PM
amckinley accepted this revision.Oct 31 2018, 7:10 PM
amckinley added inline comments.

I'm assuming it's fine to use escapeColumnName() on DB and table names?

This revision is now accepted and ready to land.Oct 31 2018, 7:10 PM

Yeah -- the actual escaping rules are the same for databases, tables, and columns. Possibly the interface should separate them or call the method escapeTableOrDatabaseOrColumnName() but it seems unlikely that we'll support an alternate SQL implementation any time too soon.

This revision was automatically updated to reflect the committed changes.