Issue with 'RecentTopics' macro on PostgreSQL DB

[ntmlod]

Not as an expert in the different SQL implementations but the syntax of get_recent_topics seems not to be compliant with psql

Log error

2017-01-17 15:24:26,276 Trac[formatter] ERROR: Macro RecentTopics(2) failed: 
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/trac/wiki/formatter.py", line 765, in _macro_formatter
    return macro.ensure_inline(macro.process(args))
  File "/usr/lib/python2.7/site-packages/trac/wiki/formatter.py", line 356, in process
    text = self.processor(text)
  File "/usr/lib/python2.7/site-packages/trac/wiki/formatter.py", line 343, in _macro_processor
    text)
  File "build/bdist.linux-x86_64/egg/tracdiscussion/wiki.py", line 102, in expand_macro
    return self._recent_topics(formatter, content)
  File "build/bdist.linux-x86_64/egg/tracdiscussion/wiki.py", line 169, in _recent_topics
    entries = self.api.get_recent_topics(context, forum_id, limit)
  File "build/bdist.linux-x86_64/egg/tracdiscussion/model.py", line 253, in get_recent_topics
    for row in self.env.db_query(sql, values)]
  File "/usr/lib/python2.7/site-packages/trac/db/api.py", line 122, in execute
    return db.execute(query, params)
  File "/usr/lib/python2.7/site-packages/trac/db/util.py", line 121, in execute
    cursor.execute(query, params)
  File "/usr/lib/python2.7/site-packages/trac/db/util.py", line 65, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
ProgrammingError: subquery in FROM must have an alias
LINE 1: SELECT forum, topic, MAX(time) as max_time  FROM  (SELECT fo...
                                                          ^
HINT:  For example, FROM (SELECT ...) [AS] foo.

My 2 cents fix

tracdiscussion/model.py

@@ -231 +231 @@
 
     def get_recent_topics(self, context, forum_id, limit):
         columns = ('forum', 'topic', 'time')
-        sql = ("SELECT forum, topic, MAX(time) as max_time"
+        sql = ("SELECT forum, topic, max(time)"
                "  FROM"
                "  (SELECT forum, topic, time"
                    " FROM message"
                "   UNION"
                "   SELECT forum, id as topic, time"
-                   " FROM topic)" +
-               (forum_id and " WHERE forum=%s" or '') +
-               " GROUP BY topic"
-               " ORDER BY max_time DESC" +
+                   " FROM topic)"
+           "  AS foo" +
+               (forum_id and " WHERE foo.forum=%s" or '') +
+               " GROUP BY foo.topic, foo.forum, foo.time"
+               " ORDER BY time DESC" +
                (limit and " LIMIT %s" or ''))
         values = []
         if forum_id:

[Ryan J Ollos]

If you added a unit test​ we could evaluate the patch for the 3 database types.

[ntmlod]

After a review, my initial patch was correct but did not give one result per topic

After several query tries, I defined this new fix

discussionplugin/1.0/tracdiscussion/model.py

@@ -231 +231 @@
 
     def get_recent_topics(self, context, forum_id, limit):
         columns = ('forum', 'topic', 'time')
-        sql = ("SELECT forum, topic, MAX(time) as max_time"
+        sql = ("SELECT forum, topic, MAX(time) as time"
                "  FROM"
                "  (SELECT forum, topic, time"
                    " FROM message"
                "   UNION"
                "   SELECT forum, id as topic, time"
-                   " FROM topic)" +
+                   " FROM topic) AS foo" +
                (forum_id and " WHERE forum=%s" or '') +
-               " GROUP BY topic"
-               " ORDER BY max_time DESC" +
+               " GROUP BY topic, forum"
+               " ORDER BY time DESC" +
                (limit and " LIMIT %s" or ''))
         values = []
         if forum_id:

It seems that we can't rename MAX aggregator differently from 'time', must set an alias for the sub-query and add 'forum' argument in GROUP BY as a consequence.