freetags 4.1: Base related entries on amount of shared tags#211
freetags 4.1: Base related entries on amount of shared tags#211
Conversation
Instead of taking all entries with common tags and to order them by date, now it orders all those entries first by the amount of common tags. That way entries that are topically related will feature more prominently in the list of related entries, if tagged accordingly
|
I like the idea 👍 However, I'm not sure if this works with all supported DBMS, some tend to be restrictive concerning |
|
Good point, I tested only on sqlite so far. I'll give this a testrun on my dev server with mysql and postres as configured by uberspace. |
|
But of course, if you can test as well or have a solution for one of these DBs, that would be great :) |
|
@mattsches, okay, got it I think! You were completely right, postgres did not like the missing columns. I added them, expecting the count to fail then, but no, everything seems to work fine. Tests still welcome ;) But less necessary now. Thanks for catching this! |
| AND e2.isdraft = 'false' | ||
| " . (!serendipity_db_bool($serendipity['showFutureEntries']) ? " AND e2.timestamp <= " . time() : '') . " | ||
| ORDER BY e2.timestamp DESC | ||
| GROUP BY e2.id, e1.entryid, e2.title, e2.timestamp |
There was a problem hiding this comment.
I think e2.id is not needed here since
- it's the same as
e1.entryidand - it's not in the
SELECT
I could only test it against SQLite, but removing it does not seem to do any harm.
There was a problem hiding this comment.
I tested it with postgres. It's correct, e2.id is not needed in the GROUP clause.
| ON e1.entryid = e2.id | ||
| WHERE e1.tag IN ('" . implode("', '", $tags) . "') | ||
| AND e1.entryid != " . (int)$postID . " | ||
| AND e2.isdraft = 'false' |
There was a problem hiding this comment.
Not sure if 'false' (as string) works everywhere as expected. The column seems to be stored as bool, so maybe query for bool or e2.isdraft IN ('false', false, 0)?
There was a problem hiding this comment.
I would prefer not to change it, as I just copied it from the old SQL and we are not aware of a problem with this. The = 'false' comparison surprisingly works in SQLite, MariaDB and PostgreSQL, I tested it now with a test draft entry :) Changing it might introduce an issue somewhere - the approach with the IN seems like it should cover the possible difference with e.g. sqlite if it stored a string in there, but NULL might become a problem.
Instead of taking all entries with common tags and to order them by date, now it orders all those entries first by the amount of common tags. That way entries that are topically related will feature more prominently in the list of related entries, if tagged accordingly