Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2]postgres and finder suggestions #44384

Merged
merged 8 commits into from
Nov 27, 2024
Merged

[5.2]postgres and finder suggestions #44384

merged 8 commits into from
Nov 27, 2024

Conversation

alikon
Copy link
Contributor

@alikon alikon commented Oct 31, 2024

Pull Request for Issue #44373
.

Summary of Changes

rewrite the query

Testing Instructions

create content with few words
be sure it indexed by Smart search component
add module Smart search to site
type word into search edit

Actual result BEFORE applying this Pull Request

No suggestions

Expected result AFTER applying this Pull Request

suggestions

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@alikon alikon marked this pull request as ready for review October 31, 2024 13:02
Copy link
Member

@richard67 richard67 left a comment

Choose a reason for hiding this comment

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

The change of this PR here is right.

We could insist now on quoteNames being used and so on, but that would not be consistent with the other code in that file where it is not used either at all places, and cleaning that up would be out of scope of this PR here.

@softarius
Copy link
Contributor

It works for me in Postgresql 16.4, MySQL 8 and MariaDB 5.6
@richard67 I'm asking you to merge this PR

@richard67
Copy link
Member

It works for me in Postgresql 16.4, MySQL 8 and MariaDB 5.6 @richard67 I'm asking you to merge this PR

@softarius Could you mark your test result in the issue tracker https://issues.joomla.org/tracker/joomla-cms/44384 by using the blue "Test this" button at the top left corner, select your test result (success) and submit? Every non-trivial PR needs 2 successful human tests, and when you mark your test result 50% is done. Thanks in advance.

@softarius
Copy link
Contributor

I have tested this item ✅ successfully on 840858f

Suceessfully tested in Postgresql 16.4, MySQL 8 and MariaDB 5.6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44384.

@richard67
Copy link
Member

@softarius Thanks for testing. Unfortunately I think this PR might not be in time for the upcoming 5.2.2 because packages for that version are already in preparation for being tested by the CMS Release Team, so it would be the next version after that, 5.2.3, which is planned for January 7, 2025. Not my responsibility or decision, I only report what I know. I will try my best to bring this PR forward, but I can't promise that it will help to bring it into 5.2.2.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 840858f

Tested with PostgreSQL 16.4 and MySQL 8.0.40.

With PostgreSQL 16 without this PR in /var/log/postgresql/postgresql-16-main.log:

2024-11-23 14:17:20.339 CET [5520] jdb2adm@joomladb2 ERROR:  for SELECT DISTINCT, ORDER BY expressions must appear in select list at character 288
2024-11-23 14:17:20.339 CET [5520] jdb2adm@joomladb2 STATEMENT:  SELECT DISTINCT(t.term)
	FROM "#__finder_terms" AS "t"
	INNER JOIN "#__finder_links_terms" AS "tm" ON tm.term_id = t.term_id
	INNER JOIN "#__finder_links" AS "l" ON (tm.link_id = l.link_id)
	WHERE t.term_id IN ($1) AND l.access IN (1,5) AND l.state = 1 AND l.published = 1
	ORDER BY t.links DESC,t.weight DESC LIMIT 10

With this PR no such log, and finder suggestions are shown.

With MySQL 8.0 with this PR it works as well as without this PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44384.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44384.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 23, 2024
@Hackwar Hackwar added the bug label Nov 23, 2024
@Hackwar Hackwar merged commit d685790 into joomla:5.2-dev Nov 27, 2024
4 checks passed
@Hackwar Hackwar added this to the Joomla! 5.2.3 milestone Nov 27, 2024
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 27, 2024
@Hackwar
Copy link
Member

Hackwar commented Nov 27, 2024

Thank you for your contribution!

@alikon alikon deleted the patch-23 branch November 27, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants