From ec931b93848f49ac5df9e3546112df34c2826f64 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 8 Sep 2019 12:59:59 -0700
Subject: [PATCH] (stable) Remove the "ONLY_FULL_GROUP_BY" SQL mode setup
 warning and change the setup key for "STRICT_ALL_TABLES"

Summary:
Ref T13404. Except for one known issue in Multimeter, Phabricator appears to function properly in this mode. It is broadly desirable that we run in this mode; it's good on its own, and enabled by default in at least some recent MySQL.

Additionally, "ONLY_FULL_GROUP_BY" and "STRICT_ALL_TABLES" shared a setup key, so ignoring one would ignore both. Change the key so that existing ignores on "ONLY_FULL_GROUP_BY" do not mask "STRICT_ALL_TABLES" warnings.

Test Plan: Grepped for `ONLY_FULL_GROUP_BY`.

Maniphest Tasks: T13404

Differential Revision: https://secure.phabricator.com/D20791
---
 .../check/PhabricatorMySQLSetupCheck.php      | 45 +------------------
 1 file changed, 1 insertion(+), 44 deletions(-)

diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
index a4048cbc33..2cbcb3cee0 100644
--- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php
+++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
@@ -76,7 +76,7 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
         phutil_tag('tt', array(), 'mysqld'),
         phutil_tag('pre', array(), 'sql_mode=STRICT_ALL_TABLES'));
 
-      $this->newIssue('mysql.mode')
+      $this->newIssue('sql_mode.strict')
         ->setName(pht('MySQL %s Mode Not Set', 'STRICT_ALL_TABLES'))
         ->setSummary($summary)
         ->setMessage($message)
@@ -84,49 +84,6 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
         ->addMySQLConfig('sql_mode');
     }
 
-    if (in_array('ONLY_FULL_GROUP_BY', $modes)) {
-      $summary = pht(
-        'MySQL is in ONLY_FULL_GROUP_BY mode (on host "%s"), but using this '.
-        'mode is strongly discouraged.',
-        $host_name);
-
-      $message = pht(
-        "On database host \"%s\", the global %s is set to %s. ".
-        "It is strongly encouraged that you disable this mode when running ".
-        "Phabricator.\n\n".
-        "With %s enabled, MySQL rejects queries for which the select list ".
-        "or (as of MySQL 5.0.23) %s list refer to nonaggregated columns ".
-        "that are not named in the %s clause. More importantly, Phabricator ".
-        "does not work properly with this mode enabled.\n\n".
-        "You can find more information about this mode (and how to configure ".
-        "it) in the MySQL manual. Usually, it is sufficient to change the %s ".
-        "in your %s file (in the %s section) and then restart %s:\n\n".
-        "%s\n".
-        "(Note that if you run other applications against the same database, ".
-        "they may not work with %s. Be careful about enabling ".
-        "it in these cases and consider migrating Phabricator to a different ".
-        "database.)",
-        $host_name,
-        phutil_tag('tt', array(), 'sql_mode'),
-        phutil_tag('tt', array(), 'ONLY_FULL_GROUP_BY'),
-        phutil_tag('tt', array(), 'ONLY_FULL_GROUP_BY'),
-        phutil_tag('tt', array(), 'HAVING'),
-        phutil_tag('tt', array(), 'GROUP BY'),
-        phutil_tag('tt', array(), 'sql_mode'),
-        phutil_tag('tt', array(), 'my.cnf'),
-        phutil_tag('tt', array(), '[mysqld]'),
-        phutil_tag('tt', array(), 'mysqld'),
-        phutil_tag('pre', array(), 'sql_mode=STRICT_ALL_TABLES'),
-        phutil_tag('tt', array(), 'ONLY_FULL_GROUP_BY'));
-
-      $this->newIssue('mysql.mode')
-        ->setName(pht('MySQL %s Mode Set', 'ONLY_FULL_GROUP_BY'))
-        ->setSummary($summary)
-        ->setMessage($message)
-        ->setDatabaseRef($ref)
-        ->addMySQLConfig('sql_mode');
-    }
-
     $is_innodb_fulltext = false;
     $is_myisam_fulltext = false;
     if ($this->shouldUseMySQLSearchEngine()) {