Skip to main content ↑ ↓
Topic: Query in getTopicsPostsAndPoster (Read 1693 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

Query in getTopicsPostsAndPoster

The query here:
https://github.com/emanuele45/Dialogo/blob/1f121a84af4ae4dac347861cc1d7373a25687a15/sources/subs/Topic.subs.php#L1950
in PostgreSQL is replaced with:
Code: [Select]
				'~GROUP BY id_msg\s+HAVING~' => 'AND',
that means the result is more or less:
Code: [Select]
		SELECT id_msg, id_member, approved
		FROM {db_prefix}messages
		WHERE id_topic = {int:current_topic}' . (!$modSettings['postmod_active'] || allowedTo('approve_posts') ? '' : (!empty($modSettings['db_mysql_group_by_fix']) ? '' : ' AND (approved = {int:is_approved}' . ($user_info['is_guest'] ? '' : ' OR id_member = {int:current_member}') . ')') . '
		ORDER BY id_msg ' . ($sort ? '' : 'DESC') . ($limit['messages_per_page'] == -1 ? '' : '
		LIMIT ' . $limit['start'] . ', ' . $limit['offset']),

Do you see any reason for the latter not to work with MySQL?

I did some quick tests and I didn't see anything, but of course I'm not entirely sure... :-[

ETA: I forgot to say that I was looking at it in order to remove if possible the PostgreSQL replacement. O:-)
Bugs creator.
Features destroyer.
Template killer.

Re: Query in getTopicsPostsAndPoster

Reply #1 –

I'm still not awake ... but what is that group by doing anyway?  msg_id is unique so what is it going to group, and there are no aggregate functions that are being done.  Need my coffee !

Re: Query in getTopicsPostsAndPoster

Reply #2 –

That was my reaction as well...
I tracked down the addition of the GROUP BY to this commit edc399f49082b425b29e5092f8fe7878ef048e24 (old SMF repo).
Commit message: ! Some databases require you to have a GROUP BY clause when using HAVING. (Display.php) [Bug 2086]
The bug is:
http://dev.simplemachines.org/mantis/view.php?id=2086

Okay, I decided to dig a bit more.
Before commit 8fb8680cbc77a6fca35a33017116f41beaf425be (old SMF repo), the code was:
Code: [Select]
		WHERE id_topic = {int:current_topic}' . (allowedTo('approve_posts') ? '' : '
			AND (approved = {int:is_approved}' . ($user_info['is_guest'] ? '' : ' OR id_member = {int:current_member}') . ')') . '
then, in the commit mentioned (message: ! Post moderation wasn't fully gone in its disabled state. (several files)), this was changed to:
Code: [Select]
		WHERE id_topic = {int:current_topic}' . (!in_array('pm', $context['admin_features']) || allowedTo('approve_posts') ? '' : '
		HAVING (approved = {int:is_approved}' . ($user_info['is_guest'] ? '' : ' OR id_member = {int:current_member}') . ')') . '
the commit changed several files, and this is the only case where the AND was replaced with an HAVING.
No idea of the reason behind.

Afterwards, since Postgre was not happy of the HAVING, the replacement was added to take care of it (and convert it to an AND).
Then, the GROUP BY was added in commit edc399f49082b425b29e5092f8fe7878ef048e24 for the already mentioned reasons (i.e. sqlite was not happy).

To me, all this screams: add back my AND and forget about the HAVING. :P
Bugs creator.
Features destroyer.
Template killer.

Re: Query in getTopicsPostsAndPoster

Reply #3 –

Curious for sure .... Thanks for digging out that linage.   So it looks like the GROUP BY by was added to avoid the error caused by using a HAVING statement (which should really refer to an aggregate value) ... Still seems like there is nothing to group, so the AND would be proper IMO
 

Re: Query in getTopicsPostsAndPoster

Reply #4 –

Thinking the HAVING would be there to use multiple indexes.