Merge pull request #11166 from annando/group-by

Issue 10935: Improved "GROUP BY" handling

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 "ANY_VALUE(...)" 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 = 'ANY_VALUE(' . $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 = '*';
 		}

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');

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(
+		$items = Post::selectToArray(['parent-uri-id', 'created', 'received'], $condition, ['group_by' => ['parent-uri-id'], 'order' => ['received' => true]]);
-			"SELECT `parent-uri-id` AS `uri-id`, MAX(`created`), MAX(`received`) FROM `post-user-view`
+		if (!DBA::isResult($items)) {
-				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)) {
 			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);

src/Worker/Notifier.php

@@ -446,26 +446,9 @@ class Notifier
 			if ($diaspora_delivery && !$unlisted) {
 				$batch_delivery = true;
 
-				$participants_stmt = DBA::p(
+				$participants = DBA::selectToArray('contact', ['batch', 'network', 'protocol', 'id', 'url', 'name'],
-					"SELECT
+					["`network` = ? AND `batch` != '' AND `uid` = ? AND `rel` != ? AND NOT `blocked` AND NOT `pending` AND NOT `archive`", Protocol::DIASPORA, $owner['uid'], Contact::SHARING],
-						`batch`, `network`, `protocol`,
+					['group_by' => ['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);
 
 				// Fetch the participation list
 				// The function will ensure that there are no duplicates