[com_finder] - SQL change for removeOrphanNodes() and postgresql driver fix #12313

alikon · 2016-10-04T21:06:41Z

Pull Request for Issue #9478 .

Summary of Changes

sql bug fix for the postgresql com_finder driver
altered fields (title, description) for #__finder_links table
removeOrphanNodes() sql rewrite and should be tested on MySQL/MariaDB too

Testing Instructions

apply the pr
on postgres only go to Extensions > Manage>Database - click on fix
enable the smart search content plugin , create/ save an article
index content on Smart Search: Indexed Content
run the cli finder_index.php script

bugfix for the postgres driver

fix for title and description fields of #__finder_links table the title filed is varchar 400 on mysql the descrpition need to be text

title and description fields for #__finder_links table

wrong file name

field title , description of #__finder_links table

this table does not exist // Optimize the terms mapping table. $db->setQuery('REINDEX TABLE ' . $db->quoteName('#__finder_links_terms')); $db->execute();

waader · 2016-10-05T15:23:43Z

I did a setup with latest staging, testing data and 4 additional languages. After installation I went in Extensions > Database and it gives me this message:

Table 'kp7cx_finder_links' does not have column 'description' with type text DEFAULT '' NOT NULL. (From file 3.6.3-2016-10-04.sql.)

So I pressed the Fix-Button and I got this syntax error:
Syntaxfehler bei »DEFAULT« LINE 1: ...inder_links" ALTER COLUMN "description" TYPE text DEFAULT ''... ^SQL=ALTER TABLE "#__finder_links" ALTER COLUMN "description" TYPE text DEFAULT '' NOT NULL;

So next I started the indexing process which went fine but at the end I got this syntax error:

FEHLER: Syntaxfehler bei »t« LINE 1: DELETE t FROM "#finder_taxonomy" AS t LEFT JOIN "#... ^SQL=DELETE t FROM "#__finder_taxonomy" AS t LEFT JOIN "#__finder_taxonomy_map" AS m ON m.node_id = t.id WHERE t.parent_id > 1 AND m.link_id IS NULL

Finally I did a search for "test" in the "Indexed Content" and this message appears (operator doesn´t exist: timestamp without timezone ...):

FEHLER: Operator existiert nicht: timestamp without time zone ~~ unknown LINE 4: ...t%' OR "l"."url" LIKE '%test%' OR "l"."indexdate" LIKE '%te... ^ HINT: Kein Operator stimmt mit dem angegebenen Namen und den Argumenttypen überein. Sie müssen möglicherweise ausdrückliche Typumwandlungen hinzufügen.SQL=SELECT l.,"t"."title" AS "t_title" FROM "#__finder_links" AS "l" INNER JOIN "#__finder_types" AS "t" ON "t"."id" = "l"."type_id" WHERE ("l"."title" LIKE '%test%' OR "l"."url" LIKE '%test%' OR "l"."indexdate" LIKE '%test%') ORDER BY l.title ASC LIMIT 20 FEHLER: Operator existiert nicht: timestamp without time zone ~~ unknown LINE 4: ...t%' OR "l"."url" LIKE '%test%' OR "l"."indexdate" LIKE '%te... ^ HINT: Kein Operator stimmt mit dem angegebenen Namen und den Argumenttypen überein. Sie müssen möglicherweise ausdrückliche Typumwandlungen hinzufügen.SQL=SELECT COUNT() FROM "#__finder_links" AS "l" INNER JOIN "#__finder_types" AS "t" ON "t"."id" = "l"."type_id" WHERE ("l"."title" LIKE '%test%' OR "l"."url" LIKE '%test%' OR "l"."indexdate" LIKE '%test%') FEHLER: Operator existiert nicht: timestamp without time zone ~~ unknown LINE 4: ...t%' OR "l"."url" LIKE '%test%' OR "l"."indexdate" LIKE '%te... ^ HINT: Kein Operator stimmt mit dem angegebenen Namen und den Argumenttypen überein. Sie müssen möglicherweise ausdrückliche Typumwandlungen hinzufügen.SQL=SELECT COUNT(*) FROM "#__finder_links" AS "l" INNER JOIN "#__finder_types" AS "t" ON "t"."id" = "l"."type_id" WHERE ("l"."title" LIKE '%test%' OR "l"."url" LIKE '%test%' OR "l"."indexdate" LIKE '%test%')

alikon · 2016-10-05T15:52:48Z

@waader the goals of this pr are more simple:

let's run index content from Smart Search: Indexed Content
let's save/create an article when the content finder plugin is enabled

a lot of more work need to be done, but this PR only affect postgresql related files so no B/C
i'll continue to work to let the com_finder workable even on Postgresql but i'll submit other PR's shortly

waader · 2016-10-05T16:11:46Z

I understand. So I repeated the test with the same setup but without the multilingual stuff.

ad 1) let's run index content from Smart Search: Indexed Content:
This brings a syntax error at the end: FEHLER: Syntaxfehler bei »t« LINE 1: DELETE t FROM "#finder_taxonomy" AS t LEFT JOIN "#... ^SQL=DELETE t FROM "#__finder_taxonomy" AS t LEFT JOIN "#__finder_taxonomy_map" AS m ON m.node_id = t.id WHERE t.parent_id > 1 AND m.link_id IS NULL

ad 2. let's save/create an article when the content finder plugin is enabled
When saving changes made to an existing article then you get this syntax error:
Save failed with the following error: FEHLER: Syntaxfehler bei »t« LINE 1: DELETE t FROM "#finder_taxonomy" AS t LEFT JOIN "#... ^SQL=DELETE t FROM "#__finder_taxonomy" AS t LEFT JOIN "#__finder_taxonomy_map" AS m ON m.node_id = t.id WHERE t.parent_id > 1 AND m.link_id IS NULL

alikon · 2016-10-05T16:16:15Z

i'm currently investigating on it .... but i'm afraid that the needed changes to let it work at least with minimal feature should'nt be only on the Postgresql side ..... i'll updated you....

remove orphans

update sql fix

typo

alikon · 2016-10-06T20:45:55Z

@waader can you retest ...
now the changes impact Mysql too, so should be tested

wilsonge · 2016-10-07T00:55:01Z

administrator/components/com_finder/helpers/indexer/taxonomy.php

@@ -329,11 +329,15 @@ public static function removeOrphanNodes()
 	{
 		// Delete all orphaned nodes.
 		$db = JFactory::getDbo();
-		$query = 'DELETE t' .
+		$query = 'DELETE ' .


Can we not make this query use JDatabaseQuery?

The entire thing would have to be rewritten as three query objects because there are a bunch of subqueries here.

use JDatabaseQuery

space

alikon · 2016-10-07T17:04:07Z

Can we not make this query use JDatabaseQuery?

no cause i'm lazy 😄
query rewritten using 3 JDatabaseQuery objects

andrepereiradasilva · 2016-10-07T17:46:57Z

administrator/components/com_finder/helpers/indexer/taxonomy.php

+
+		$subquery1->clear()
+			->select($db->quoteName('t.id'))
+			->from($db->quoteName('#__finder_taxonomy') . ' AS t')


->from($db->quoteName('#__finder_taxonomy', 't')) 😉

for the ON ?
->join('LEFT',$db->quoteName('#__finder_taxonomy_map', 'm') . 'ON ' ....)

yes, i think so, but care the space before ON :)

alias

chrisdavenport · 2016-10-08T20:39:38Z

administrator/components/com_finder/helpers/indexer/driver/postgresql.php

@@ -416,7 +416,7 @@ public function index($item, $format = 'html')
 				' ROUND(SUM(' . $db->quoteName('context_weight') . '), 8)' .
 				' FROM ' . $db->quoteName('#__finder_tokens_aggregate') .
 				' WHERE ' . $db->quoteName('map_suffix') . ' = ' . $db->quote($suffix) .
-				' GROUP BY ' . $db->quoteName('term') .
+				' GROUP BY ' . $db->quoteName('term') . ' ,' . $db->quoteName('term_id') .


', ' instead of ' ,' ?

chrisdavenport · 2016-10-08T20:44:27Z

administrator/components/com_finder/helpers/indexer/driver/postgresql.php

-		// Optimize the terms mapping table.
-		$db->setQuery('REINDEX TABLE ' . $db->quoteName('#__finder_links_terms'));
-		$db->execute();
-


As per my comment on your other PR, I think this should be #__finder_terms and so should be fixed rather than removed. Also, for completeness, there are other tables that should be optimised at the same time.

chrisdavenport · 2016-10-08T20:46:56Z

administrator/components/com_finder/helpers/indexer/taxonomy.php

+		$subquery  = $db->getQuery(true);
+		$subquery1 = $db->getQuery(true);
+
+		$subquery1->clear()


I don't think it's necessary to call clear() here because you get a fresh query object from getQuery(true) anyway. Same for the next two statements.

Clear removes creating a new class instance. It's performance better to call clear than to get a fresh query object

True, but there's no point in calling clear on fresh query objects, which is what is being done here.

Oh oops. My bad!

chrisdavenport · 2016-10-08T20:49:33Z

administrator/components/com_finder/helpers/indexer/taxonomy.php

-			' AND m.link_id IS NULL';
+		$query     = $db->getQuery(true);
+		$subquery  = $db->getQuery(true);
+		$subquery1 = $db->getQuery(true);


Personally, I prefer the fluent style so I'd merge these three statements into the statements below.

reindex missing tables same as for mysql driver

removed unnecessary clear()

alikon · 2016-10-15T06:24:25Z

added the missed REINDEX
removed unnecessary clear()

No space found after comma in function call

chrisdavenport · 2016-10-18T09:29:51Z

I have not tested this item.

I've tested with MySQL and it works fine. I haven't tested on PostGres which is why I'm not marking this as a successful test.

I created a category called "Test", then created an article in that category.
I checked that a taxonomy for the Test category had been created and the article was correctly indexed.
I deleted the article (and emptied the trash). This would trigger the removeOrphanNodes method to remove the Test category taxonomy.
I did not get any errors, the Test category taxonomy had been removed and the index had been updated correctly.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12313.}

csthomas · 2017-02-01T20:38:44Z

I have tested this item ✅ successfully on fe8a684

I tested successful on PostgresSQL based on test instruction. Button Index and clear index also works.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12313.}

waader · 2017-02-02T09:43:33Z

I have tested this item ✅ successfully on fe8a684

Works with postgres, thanks Alikon!

installation/sql/postgresql/joomla.sql needs an update.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12313.}

csthomas · 2017-02-02T10:14:25Z

administrator/components/com_admin/sql/updates/postgresql/3.6.3-2016-10-04.sql

@@ -0,0 +1,3 @@
+ALTER TABLE "#__finder_links" ALTER COLUMN "title" TYPE character varying(400);
+ALTER TABLE "#__finder_links" ALTER COLUMN "description" TYPE text;
+ALTER TABLE "#__finder_links" ALTER COLUMN "description" SET NOT NULL;


What about default value '' or not for description?

infograf768 · 2017-02-16T09:54:25Z

RTC as 2 good testers.

Note: last comment is
"installation/sql/postgresql/joomla.sql needs an update."

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12313.}

alikon added 5 commits October 4, 2016 22:50

[postgresql] - com_finder index content bugfix

167b19d

bugfix for the postgres driver

fix for title and description fields of #__finder_links table

336b215

fix for title and description fields of #__finder_links table the title filed is varchar 400 on mysql the descrpition need to be text

update fix

031fdb6

title and description fields for #__finder_links table

wrong file name

5453bb0

wrong file name

fix for update

f449ac3

field title , description of #__finder_links table

joomla-cms-bot added the PR-staging label Oct 4, 2016

remove unnecessary commented code

73d639b

this table does not exist // Optimize the terms mapping table. $db->setQuery('REINDEX TABLE ' . $db->quoteName('#__finder_links_terms')); $db->execute();

alikon added 4 commits October 6, 2016 21:27

remove orphans

e6b0b37

remove orphans

remove orphans

a9754a3

remove orphans

update sql fix

7c7dc28

update sql fix

typo

cc26f1f

typo

alikon changed the title ~~[postgresql] - com_finder index content fix~~ [com_finder] - SQL change for removeOrphanNodes() and postgresql driver fix Oct 6, 2016

wilsonge reviewed Oct 7, 2016

View reviewed changes

alikon added 2 commits October 7, 2016 18:47

use JDatabaseQuery

a5bac81

use JDatabaseQuery

space

13b2586

space

andrepereiradasilva reviewed Oct 7, 2016

View reviewed changes

alikon added 2 commits October 7, 2016 20:03

alias

b659b0a

alias

alias

cc7b01a

alias

chrisdavenport reviewed Oct 8, 2016

View reviewed changes

alikon added 2 commits October 15, 2016 08:19

added requested changes

a2ef053

reindex missing tables same as for mysql driver

removed unnecessary clear()

2716513

removed unnecessary clear()

No space found after comma in function call

fe8a684

No space found after comma in function call

csthomas mentioned this pull request Feb 1, 2017

Errors in Smart search component on PostgreSQL #13848

Closed

csthomas reviewed Feb 2, 2017

View reviewed changes

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 16, 2017

zero-24 added this to the Joomla 3.7.0 milestone Feb 21, 2017

rdeutz merged commit 7fb1491 into joomla:staging Feb 22, 2017

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 22, 2017

alikon deleted the patch-81 branch February 27, 2017 11:05

csthomas mentioned this pull request Mar 19, 2017

Smart Search - sync mysql with postgresql database #14782

Merged

csthomas mentioned this pull request Sep 18, 2018

Invalid SQL created by removeOrphanNodes function #22231

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[com_finder] - SQL change for removeOrphanNodes() and postgresql driver fix #12313

[com_finder] - SQL change for removeOrphanNodes() and postgresql driver fix #12313

alikon commented Oct 4, 2016 •

edited by joomla-cms-bot

Loading

waader commented Oct 5, 2016

alikon commented Oct 5, 2016

waader commented Oct 5, 2016 •

edited

Loading

alikon commented Oct 5, 2016 •

edited

Loading

alikon commented Oct 6, 2016

wilsonge Oct 7, 2016

mbabker Oct 7, 2016

alikon commented Oct 7, 2016 •

edited

Loading

andrepereiradasilva Oct 7, 2016

alikon Oct 7, 2016

andrepereiradasilva Oct 7, 2016

alikon Oct 8, 2016

chrisdavenport Oct 8, 2016

chrisdavenport Oct 8, 2016

chrisdavenport Oct 8, 2016 •

edited

Loading

wilsonge Oct 8, 2016

chrisdavenport Oct 8, 2016

wilsonge Oct 9, 2016

chrisdavenport Oct 8, 2016

alikon commented Oct 15, 2016

chrisdavenport commented Oct 18, 2016

csthomas commented Feb 1, 2017 •

edited

Loading

waader commented Feb 2, 2017

csthomas Feb 2, 2017

infograf768 commented Feb 16, 2017

[com_finder] - SQL change for removeOrphanNodes() and postgresql driver fix #12313

[com_finder] - SQL change for removeOrphanNodes() and postgresql driver fix #12313

Conversation

alikon commented Oct 4, 2016 • edited by joomla-cms-bot Loading

Summary of Changes

Testing Instructions

waader commented Oct 5, 2016

alikon commented Oct 5, 2016

waader commented Oct 5, 2016 • edited Loading

alikon commented Oct 5, 2016 • edited Loading

alikon commented Oct 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alikon commented Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisdavenport Oct 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alikon commented Oct 15, 2016

chrisdavenport commented Oct 18, 2016

csthomas commented Feb 1, 2017 • edited Loading

waader commented Feb 2, 2017

Choose a reason for hiding this comment

infograf768 commented Feb 16, 2017

alikon commented Oct 4, 2016 •

edited by joomla-cms-bot

Loading

waader commented Oct 5, 2016 •

edited

Loading

alikon commented Oct 5, 2016 •

edited

Loading

alikon commented Oct 7, 2016 •

edited

Loading

chrisdavenport Oct 8, 2016 •

edited

Loading

csthomas commented Feb 1, 2017 •

edited

Loading