Page MenuHomePhabricator

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

Authored by epriestley on Oct 26 2018, 3:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 11:16 AM
Unknown Object (File)
Sun, Nov 3, 1:53 PM
Unknown Object (File)
Oct 23 2024, 4:40 AM
Unknown Object (File)
Oct 18 2024, 7:50 PM
Unknown Object (File)
Oct 10 2024, 4:50 AM
Unknown Object (File)
Oct 9 2024, 10:28 AM
Unknown Object (File)
Sep 23 2024, 9:28 AM
Unknown Object (File)
Sep 12 2024, 5:27 PM
Subscribers
None

Details

Summary

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 abc.xyz, 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 "abc.xyz".

Test Plan

See the next change for callsites and unit tests.

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/xsprintf/qsprintf.php
242

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.