qsprintf() now returns a PhutilQueryString object instead of a raw string. This affects construction of some queries with qsprintf(), and execution of some queries with queryfx(), queryfx_one(), queryfx_all(), LiskDAO->loadOneWhere(...) and LiskDAO->loadAllWhere(...).

This makes unsafe query construction more difficult and supports a %P (password/secret) conversion. See T6960.

This change primarily affects custom code which uses %Q. If you pass a raw string to %Q, you will now get a warning:

queryfx_all(
  $conn,
  'SELECT * FROM ... WHERE %Q',
  'id > 3');

PHLOG: 'UNSAFE: Raw string ("id > 3") passed to query ("SELECT * FROM ... WHERE %Q") for "%Q" conversion. %Q should be passed a query string.' at ...

If you see this warning in custom code: Your custom code should be updated. See the rest of this task for guidance.

If you see this warning in upstream code: First, make sure you're up to date! Whatever you're seeing may already have been fixed. If it reproduces with the most recent version, You can report any warnings you run across as bugs on Discourse.

Guide for Updating Custom Code

Instead of passing strings to %Q as above, build the query subcomponent with qsprintf():

queryfx_all(
  $conn,
  'SELECT * FROM ... WHERE %Q',
  qsprintf($conn, 'id > %d', 3));

Practically, it's unlikely you were doing any of this, since it's easier to write WHERE id > %d in the first place. However, you may have used %Q to combine subclauses with AND or OR, or to combine value subclauses (like (X, Y)) for INSERT .... In these cases, you would generally pass implode( ... ) to queryfx*(). These can now be rewritten with:

• %LA: List of subclauses joined by AND: X, Y -> ((X) AND (Y))
• %LO: List of subclauses joined by OR: X, Y -> ((X) OR (Y))
• %LQ: List of subclauses joined by , (comma): X, Y -> X, Y (useful for composing INSERT INTO ... VALUES ...).
• %LJ: List of subclauses joined by (space): X, Y -> X Y (useful for composing JOIN clauses).

For example, if you had code like this which used implode():

$where[] = qsprintf(...);
$where[] = qsprintf(...);

...

queryfx_all(
  $conn,
  'SELECT ... WHERE %Q',
  implode(' AND ', $where));

...it can now be rewritten with %LA instead:

$where[] = qsprintf(...);
$where[] = qsprintf(...);

...

queryfx_all(
  $conn,
  'SELECT ... WHERE %LA',
  $where);

Likewise, if you had code like this:

queryfx(
  $conn,
  'INSERT INTO ... (x, y) VALUES %Q',
  implode(', ', $chunk_of_values));

...it can now be rewritten with %LQ instead:

queryfx(
  $conn,
  'INSERT INTO ... (x, y) VALUES %LQ',
  $chunk_of_values);

Concurrently, there are a few other qsprintf() changes to be aware of:

• The new %P conversion can be used to build queries with passwords or secrets. It will prevent the value from printing in logs, --trace, etc.
• The new %R (database + table ref) conversion should generally replace %T (table). See T11908 for discussion.
• The PhabricatorLiskDAO::chunkSQL() method has changed behavior. It no longer accepts a joiner string and now returns a list. In most cases, the correct change for this new behavior should be to replace %Q with %LQ.
• The signatures of some PhabricatorQuery building blocks have changed. These methods generally either now accept a $conn parameter or no longer exist (because %LA, %LO, or similar provide the same behavior). Newer code is likely unaffected since direct use of these methods is less conventional in modern code than it was in the past.
• On AphrontDatabaseConnection, executeRawQuery($raw_query) is now executeQuery(PhutilQueryString $query). It is unlikely any third-party code is affected by this change.
• The new %LK conversion is a very niche conversion for creating and altering keys on tables. It functions like %LC but handles columname(32) properly.

In a small number of cases, a %... conversion may not exist. We currently have two (a dynamic number of clauses joined by UNION DISTINCT, and a list of fields being summed). You can "implode" these unusual queries by building the query strings step by step:

$unions = null;
foreach ($selects as $select) {
  if (!$unions) {
    $unions = $select;
    continue;
  }

  $unions = qsprintf(
    $conn,
    '%Q UNION DISTINCT %Q',
    $unions,
    $select);
}

In extreme cases, the new %Z conversion behaves like the old %Q conversion did and allows you to use arbitrary query text. We currently have two of these: one for running .sql migrations (which are arbitrary text) and one for column types in bin/storage adjust (which can be removed eventually, it's just a lot of work to hard-code a big list of valid column types).