Issue 10935: Improved "GROUP BY" handling

diff --git a/src/Database/Database.php b/src/Database/Database.php
index a812efbdb9a9f09811118e389bd459fe96d7fc97..292625bb5722ccfdd8abad660f1f1f88ff4f1cb4 100644 (file)
--- a/src/Database/Database.php
+++ b/src/Database/Database.php
@@ -1367,6 +1367,45 @@ class Database
                return $this->toArray($this->select($table, $fields, $condition, $params));
        }
 
+       /**
+        * Escape fields, adding special treatment for "group by" handling
+        *
+        * @param array $fields 
+        * @param array $options 
+        * @return array 
+        */
+       private function escapeFields(array $fields, array $options)
+       {
+               // In the case of a "GROUP BY" we have to add all the ORDER fields to the fieldlist.
+               // This needs to done to apply the "MAX(...)" treatment from below to them.
+               // Otherwise MySQL would report errors.
+               if (!empty($options['group_by']) && !empty($options['order'])) {
+                       foreach ($options['order'] as $key => $field) {
+                               if (!is_int($key)) {
+                                       if (!in_array($key, $fields)) {
+                                               $fields[] = $key;
+                                       }
+                               } else {
+                                       if (!in_array($field, $fields)) {
+                                               $fields[] = $field;
+                                       }
+                               }
+                       }
+               }
+
+               array_walk($fields, function(&$value, $key) use ($options)
+               {
+                       $field = $value;
+                       $value = '`' . str_replace('`', '``', $value) . '`';
+
+                       if (!empty($options['group_by']) && !in_array($field, $options['group_by'])) {
+                               $value = 'MAX(' . $value . ') AS ' . $value;
+                       }
+               });
+
+               return $fields;
+       }
+
        /**
         * Select rows from a table
         *
@@ -1403,7 +1442,8 @@ class Database
                }
 
                if (count($fields) > 0) {
-                       $select_string = implode(', ', array_map([DBA::class, 'quoteIdentifier'], $fields));
+                       $fields = $this->escapeFields($fields, $params);
+                       $select_string = implode(', ', $fields);
                } else {
                        $select_string = '*';
                }

diff --git a/src/Module/Api/Friendica/Photo/Create.php b/src/Module/Api/Friendica/Photo/Create.php
index 62889d328c9acbd3d504cd7099f1ebaa56b4668d..bb3dc63b2280329eb7194dde56cb7e7e7bb023f0 100644 (file)
--- a/src/Module/Api/Friendica/Photo/Create.php
+++ b/src/Module/Api/Friendica/Photo/Create.php
@@ -55,7 +55,7 @@ class Create extends BaseApi
                $type = $this->getRequestValue($this->parameters, 'extension', 'json');
 
                // input params
-               $desc      = $this->getRequestValue($request, 'desc') ?? '';
+               $desc      = $this->getRequestValue($request, 'desc', '');
                $album     = $this->getRequestValue($request, 'album');
                $allow_cid = $this->getRequestValue($request, 'allow_cid');
                $deny_cid  = $this->getRequestValue($request, 'deny_cid');

diff --git a/src/Module/Update/Profile.php b/src/Module/Update/Profile.php
index 22e6e4a997b1e371dd57624cb31cdfa7981f1d2c..b97b9f89e5d955a9c8286e8d20a0e3843343a477 100644 (file)
--- a/src/Module/Update/Profile.php
+++ b/src/Module/Update/Profile.php
@@ -70,32 +70,35 @@ class Profile extends BaseModule
 
                $last_updated = $last_updated_array[$last_updated_key] ?? 0;
 
+               $condition = ["`uid` = ? AND NOT `contact-blocked` AND NOT `contact-pending`
+                               AND `visible` AND (NOT `deleted` OR `gravity` = ?)
+                               AND `wall` " . $sql_extra, $a->getProfileOwner(), GRAVITY_ACTIVITY];
+
                if ($_GET['force'] && !empty($_GET['item'])) {
                        // When the parent is provided, we only fetch this
-                       $sql_extra4 = " AND `parent` = " . intval($_GET['item']);
+                       $condition = DBA::mergeConditions($condition, ['parent' => $_GET['item']]);
                } elseif ($is_owner || !$last_updated) {
                        // If the page user is the owner of the page we should query for unseen
                        // items. Otherwise use a timestamp of the last succesful update request.
-                       $sql_extra4 = " AND `unseen`";
+                       $condition = DBA::mergeConditions($condition, ['unseen' => true]);
                } else {
                        $gmupdate = gmdate(DateTimeFormat::MYSQL, $last_updated);
-                       $sql_extra4 = " AND `received` > '" . $gmupdate . "'";
+                       $condition = DBA::mergeConditions($condition, ["`received` > ?", $gmupdate]);
                }
 
-               $items_stmt = DBA::p(
-                       "SELECT `parent-uri-id` AS `uri-id`, MAX(`created`), MAX(`received`) FROM `post-user-view`
-                               WHERE `uid` = ? AND NOT `contact-blocked` AND NOT `contact-pending`
-                               AND `visible` AND (NOT `deleted` OR `gravity` = ?)
-                               AND `wall` $sql_extra4 $sql_extra
-                       GROUP BY `parent-uri-id` ORDER BY `received` DESC",
-                       $a->getProfileOwner(),
-                       GRAVITY_ACTIVITY
-               );
-
-               if (!DBA::isResult($items_stmt)) {
+               $items = Post::selectToArray(['parent-uri-id', 'created', 'received'], $condition, ['group_by' => ['parent-uri-id'], 'order' => ['received' => true]]);
+               if (!DBA::isResult($items)) {
                        return;
                }
 
+               // @todo the DBA functions should handle "SELECT field AS alias" in the future,
+               // so that this workaround here could be removed.
+               $items = array_map(function ($item) {
+                       $item['uri-id'] = $item['parent-uri-id'];
+                       unset($item['parent-uri-id']);
+                       return $item;
+               }, $items);
+
                // Set a time stamp for this page. We will make use of it when we
                // search for new items (update routine)
                $last_updated_array[$last_updated_key] = time();
@@ -113,8 +116,6 @@ class Profile extends BaseModule
                        }
                }
 
-               $items = DBA::toArray($items_stmt);
-
                $o .= DI::conversation()->create($items, 'profile', $a->getProfileOwner(), false, 'received', $a->getProfileOwner());
 
                System::htmlUpdateExit($o);

diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php
index bb66270860ec79d81f844a8533b7f23b640f735a..f46b1b0d9a3bd3f3c7048e77bead9ded198d985d 100644 (file)
--- a/src/Worker/Notifier.php
+++ b/src/Worker/Notifier.php
@@ -446,26 +446,9 @@ class Notifier
                        if ($diaspora_delivery && !$unlisted) {
                                $batch_delivery = true;
 
-                               $participants_stmt = DBA::p(
-                                       "SELECT
-                                               `batch`, `network`, `protocol`,
-                                               ANY_VALUE(`id`) AS `id`,
-                                               ANY_VALUE(`url`) AS `url`,
-                                               ANY_VALUE(`name`) AS `name`
-                                       FROM `contact`
-                                       WHERE `network` = ?
-                                       AND `batch` != ''
-                                       AND `uid` = ?
-                                       AND `rel` != ?
-                                       AND NOT `blocked`
-                                       AND NOT `pending`
-                                       AND NOT `archive`
-                                       GROUP BY `batch`, `network`, `protocol`",
-                                       Protocol::DIASPORA,
-                                       $owner['uid'],
-                                       Contact::SHARING
-                               );
-                               $participants = DBA::toArray($participants_stmt);
+                               $participants = DBA::selectToArray('contact', ['batch', 'network', 'protocol', 'id', 'url', 'name'],
+                                       ["`network` = ? AND `batch` != '' AND `uid` = ? AND `rel` != ? AND NOT `blocked` AND NOT `pending` AND NOT `archive`", Protocol::DIASPORA, $owner['uid'], Contact::SHARING],
+                                       ['group_by' => ['batch', 'network', 'protocol']]);
 
                                // Fetch the participation list
                                // The function will ensure that there are no duplicates