Skip to content

chore(queries): avoid left join when possible#3317

Merged
Altahrim merged 1 commit intomasterfrom
chore/avoid-leftjoins
Nov 19, 2025
Merged

chore(queries): avoid left join when possible#3317
Altahrim merged 1 commit intomasterfrom
chore/avoid-leftjoins

Conversation

@Altahrim
Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>
@Altahrim Altahrim added this to the Nextcloud 33 milestone Nov 19, 2025
@Altahrim Altahrim requested a review from artonge November 19, 2025 10:36
@Altahrim Altahrim self-assigned this Nov 19, 2025
@Altahrim Altahrim added php PHP related ticket 2. developing Work in progress performance 🚀 Performance issues and optimisations labels Nov 19, 2025
Copy link
Copy Markdown
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Commented on all the changes. Overall, should have limited impact, but it is safe and cleans up the code a bit so let's merge 👍

->innerJoin('c', 'photos_albums', 'a', $query->expr()->eq('a.album_id', 'c.album_id'))
->where($query->expr()->eq('collaborator_id', $query->createNamedParameter($collaboratorId)))
->andWhere($query->expr()->eq('collaborator_type', $query->createNamedParameter($collaboratorType, IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->isNotNull('a.album_id'))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably a left over from copying and pasting as album_id should always be set in the albums table.
So should be a safe cleanup 👍

->selectAlias('a.user', 'album_user')
->from('photos_albums_collabs', 'c')
->leftJoin('c', 'photos_albums', 'a', $query->expr()->eq('a.album_id', 'c.album_id'))
->innerJoin('c', 'photos_albums', 'a', $query->expr()->eq('a.album_id', 'c.album_id'))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As said, probably zero impact, as both should always be defined. But at least it is explicit.

->from('filecache', 'file');
$metadataQuery = $this->filesMetadataManager->getMetadataQuery($qb, 'file', 'fileid');
$metadataQuery->joinIndex(PlaceMetadataProvider::METADATA_KEY);
$metadataQuery->joinIndex(PlaceMetadataProvider::METADATA_KEY, true);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As discussed, depending on the DB the optimisation might already be done. But good anyway.

@Altahrim Altahrim marked this pull request as ready for review November 19, 2025 11:04
@Altahrim Altahrim added 3. to review Waiting for reviews and removed 2. developing Work in progress 3. to review Waiting for reviews labels Nov 19, 2025
@Altahrim Altahrim merged commit cfd06cd into master Nov 19, 2025
53 checks passed
@Altahrim Altahrim deleted the chore/avoid-leftjoins branch November 19, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance 🚀 Performance issues and optimisations php PHP related ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants