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)
Thu, Apr 25, 2:26 AM
Unknown Object (File)
Mon, Apr 22, 7:09 PM
Unknown Object (File)
Fri, Apr 19, 8:03 PM
Unknown Object (File)
Wed, Apr 17, 7:59 AM
Unknown Object (File)
Mar 20 2024, 10:31 PM
Unknown Object (File)
Mar 20 2024, 10:31 PM
Unknown Object (File)
Mar 20 2024, 10:31 PM
Unknown Object (File)
Mar 20 2024, 10:31 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
Branch
query1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21054
Build 28608: Run Core Tests
Build 28607: arc lint + arc unit

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.