Skip to content

freetags 4.1: Base related entries on amount of shared tags#211

Open
onli wants to merge 3 commits intomasterfrom
feature/freetagsRelated
Open

freetags 4.1: Base related entries on amount of shared tags#211
onli wants to merge 3 commits intomasterfrom
feature/freetagsRelated

Conversation

@onli
Copy link
Member

@onli onli commented Feb 20, 2026

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

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
@mattsches
Copy link
Member

I like the idea 👍

However, I'm not sure if this works with all supported DBMS, some tend to be restrictive concerning GROUP BY clauses (there is the ONLY_FULL_GROUP_BY sql_mode in MySQL, and PostgreSQL might complain about missing columns in the GROUP BY clause, etc…).

@onli
Copy link
Member Author

onli commented Feb 24, 2026

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.

@onli
Copy link
Member Author

onli commented Feb 24, 2026

But of course, if you can test as well or have a solution for one of these DBs, that would be great :)

@onli
Copy link
Member Author

onli commented Feb 24, 2026

@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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think e2.id is not needed here since

  1. it's the same as e1.entryid and
  2. it's not in the SELECT

I could only test it against SQLite, but removing it does not seem to do any harm.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants