test was made on postgresql
i've applyied the patch on a fresh install and then i've runned the Testing Sample Data module/plugin
2019-09-23 18:46:13.954 CEST [4912] postgres@joomla ERROR: invalid input syntax for integer: "" at character 57 2019-09-23 18:46:13.954 CEST [4912] postgres@joomla STATEMENT: SELECT * FROM "my4_finder_taxonomy" WHERE "parent_id" = '' AND "title" = 'Scenery' AND "language" = ''

hoping it helps to discover the root of the bug

@alikon Thank you for this, I will definitely fix that, however that bug is unrelated to this PR/bug. Could you open a new issue for that and mention me there?

Seems as if this currently fixes the bug, but the number of articles results in a maxexecution time. Maybe someone could split up the article step further?

I still get an error after this patch on current 4.0-dev, but this time the error is readable

Seems as if this currently fixes the bug, but the number of articles results in a maxexecution time. Maybe someone could split up the article step further?

with only 60 articles?... wow

I have tested this item ? unsuccessfully on 4bc43b8

post:

I'm with @N6REJ - I had no issues with execution times etc. But it doesn't give any change in the error message I'm receiving either - still the same _getNode issue (interestly mine is reliably with an id of 14 rather than his 313) :( My taxonomy table doesn't seem any different either - so I can only really conclude that this PR hasn't had any effect on the issue

@Hackwar what's your plans here? Given this has been tested to not work as it stands even when the query does run

I will investigate this in the coming week and provide a fix. To be honest, I'm a little bit stumped because this is the fix that my customer got and it works there. I will have to search what I missed there. The source of the problem however is right.

Thanks :) I can completely believe this is a decent part of the fix :)

So I found the issue, but I'm unsure how to fix it... First of all, there was a stupid little coding error, which I fixed above. But here comes the big issue:

The indexer calls the Taxonomy class to store taxonomy entries. In the store method, the code caches the used taxonomies to quickly retrieve the IDs. So now the model fires "onContentAfterSave" and the indexer runs, deletes the previous entry and indexes the new data. So far so "good". (Not exactly, but that is how it works right now...)

At some point we added onContentStateChange in a way, that it is executed when the testing sampledata plugin adds its articles. In that event, finder deletes the previous entry and reindexes it, to just then fire onContentAfterSave, deleting the entry again and doing all that indexing AGAIN, but now, since the taxonomies are identical, it runs into the issue of the cache. The previous delete also removed the then empty taxonomies, but they are still cached and thus the next indexing returns the wrong IDs and we have this nice issue here...

This also brings up the issue, that the filters, which you can create, are not going to work when the taxonomies are recreated due to such an edit. So we have to fix that one, too.

And while I'm writing all of this, I've come up with a solution. ;-) First of all, the removal of the empty taxonomies is delayed until after the reindexing is done and then the article model is recreated every time in the testing sampledata plugin to prevent it from thinking it changed state. (I did not check why that is in the model. Maybe there is a bug? Maybe we need a reset() method for the model to prevent situations like this?)

Looking at the other sampledata plugins, we need to refactor those to work similarly to the testing plugin, since those partially just insert directly into the DB.

Could those who tested thios prior, test this again?

Testing instructions would be helpful.

• Install a current 4.0 and install the testing sampledata.
• Notice that it errors out
• Apply this patch, reinstall the installation
• Notice that it does not fail anymore.

Normally we have testing instructions in the PR's description and not in some comment.

Important hint for testers: In addition to this PR, PR #27008 has to be applied, too, to make testing sample data installation work.

I have tested this item ✅ successfully on 5467182

Important note for other testers: It needs PR #27008 to be applied, too, in addition to this PR here.

Test as follows:

1. On a clean 4.0-dev installation without any sample data apply PR #27008 but not this PR here.

2. Install testing sample data.

Result: Testing sample data installation fails at step 4 with an error similar to the one shown in this comment: #26392 (comment).

3. Again on a clean 4.0-dev installation without any sample data apply PR #27008 and this PR here.

4. Install testing sample data.

Result: Testing sample data installation finishs with success.

@Hackwar Please add my instructions to a section "Testing Instructions" in the description of this PR so other testers easily find it. Otherwise we might get bad tests, e.g. because people don't apply PR #27008 , too.

@alikon @N6REJ If you want to test this PR, please use the testing instructions I gave in my test result here: #26392 (comment). Reason is that it needs PR #27008 to be applied, too, in addition to this one, and so it also needs to test that PR #27008 alone does not solve the issue with testing sample data and that it needs this PR here, too.

Also installed #27008. In the PHP error log:

PHP Fatal error: Maximum execution time of 30 seconds exceeded in C:\xampp\htdocs\joomla-cms-4.0-dev\libraries\vendor\joomla\database\src\Mysqli\MysqliStatement.php on line 443

@Quy Strange, my max_execution_time is also 30 seconds and I did not have this problem. I had a clean installation of current 4.0-dev without any other sample data or additional stuff.

Update: Have you tested with a multilingual site? Mine was not. Maybe that's the difference? Or what else could it be? Windows/Linux? Mine is Linux.

Local clean install with no other sample data on Windows 10 using XAMPP. No multilingual site.

I guess we both have MySQL 5.7? Or do you already use 8? If 5.7, then the only difference is the OS, it seems. Or maybe it is a random thing and it happens to me, too, when I test again? I will check tomorrow.

10.4.6-MariaDB

Then it would be interesting to know if someone else with same MariaDB version has the problem, too.

Hint for other testers: PR #27008 is merged now, and this PR has been updated by last changes in 4.0-dev, so there is no need anymore to apply PR #27008 for testing.

So I'm running 10.3.16-MariaDB and this patch worked for me BUT that step that you timed out on took 18 seconds on my mid-range MacBook. I can totally believe on some setups it's going to exceed 30. We'll need to spend some time figuring out exactly what's taking so long. However I think this fixes the actual error on sample data install so I'm merging this for now. @Quy could you raise an issue for this please so we can investigate the reason it's taking so long on that step to create all the articles. Thanks!

if batch inserting few articles on smart search like the sample plugin do, took 18 seconds ... then something is really bad/wrong

There's 68 articles in the test data set plus all the associated permission to calculate and insert into the asset table. I mean it's going to take a while. But yeah 18 seconds seems too long.

Either way it's something to be looked at - but at least with this we can now install sample data again :)