Today, all data (except null?) comes out of MySQL as strings, regardless of the column type in MySQL:

<?php

require_once 'scripts/__init_script__.php';

$conn = id(new PhabricatorUser())->establishConnection('r');

$result = queryfx_all(
  $conn,
  'SELECT 1, 2+2, 3/2');

var_dump($result);

epriestley@orbital ~/dev/phabricator $ php -f test.php 
array(1) {
  [0]=>
  array(3) {
    [1]=>
    string(1) "1"
    ["2+2"]=>
    string(1) "4"
    ["3/2"]=>
    string(6) "1.5000"
  }
}

This causes us to do a whole lot of explicit type juggling later on, particularly when applying transactions to integer-valued fields. This is further muddied by MySQL not having a true boolean field.

A brief attempt to tackle this problem was made in PhabricatorUser->readField() but this didn't really gain traction and is now a weird one-off piece of magic.

"Everything is a string" is basically just how the PHP layer for MySQL works. At Facebook, I recall that Brian Shire developed something to fix this as part of APE circa 2007, but I don't think it made it upstream to mainline PHP, and I don't remember what form it took (maybe "just do the right thing by default").

There are various ways we may be able to fix this ourselves in the query layer, notably:

• MYSQLI_OPT_INT_AND_FLOAT_NATIVE
• mysql[i]_fetch_field()
• mysql_field_type() -- wrapper around the above, not available for MySQLi?

It isn't clear if these work, how they work, what versions they're available on, or what other immediate query-layer problems they might cause. However, they may be viable pathways forward. If they do work, these are probably the largest component of a reasonable approach to sorting this out.

A complication here is that MySQL does not have a real "bool" type (it's an alias for TINYINT(1), so even if we correct this, boolean-valued fields will require some juggling in PHP since they'd come out of the MySQL driver layer as 0 and 1 (a big step up from "0" and "1", but still an issue).

We could fix this in most cases in the Lisk layer, as Lisk now knows about the PHP-level types of fields, and could automatically apply appropriate casts to convert 0 and 1 to false and true.

A related minor issue is that passing false to %ns ("nullable string") prints "", not "0", and passing it to %d fatals. We work around this with a lot of (int) casting, but this could reasonably be changed in the qsprintf() layer and/or Lisk layer to have better behavior.

Related work:

• T11908 discusses introducing a new %R conversion for "full database + table names" to let us reduce the number of connections we hold open.
• T6960 discusses supporting %P in qsprintf() to keep passwords out of --trace.
• T6960 also discusses a worthy effort to remove %Q from the codebase. Other changes in how queries are built have largely driven this organically by reducing the number of cases where we need %Q, but the goal is also worth considering.

Any change here carries a substantial risk of causing far-reaching, hard-to-detect bugs, because the query layer would be so fundamentally affected and changing data types from string to int usually does not result in obvious failures. Two future changes which would directly benefit from fixing this are:

• I expect to eventually introduce a transaction.search or similar, for T5873. This will be easier if our internal transaction storage uses basically-sane types, so we aren't spewing a weird mixture of "0", 0, null and false out over the API. (However, this might be mooted because we may want a per-transaction getDataForConduit() sort of method anyway, which could do appropriate casting.)
• It would be desirable to fix this before putting resources toward support for an application ecosystem (T5447) so we don't have to explain it to third-party developers.

We could take some steps to mitigate impact here, like do the type casting only in developer mode for a while.

In theory, all of the code already does all the requisite casts for strings, and those casts should produce the same results for integers or booleans. That is, the (int) and (bool) of false, 0, and "0" are all the same. If the code can already accept "0" -- the most clearly wrong of the three -- it follows that we may not be heading into too much peril by replacing this with a "better" type.

But this could do a lot of weird stuff that takes a very long time to chase down -- and it would be particularly difficult for us to test if historic data migrations are affected.