Delete old search entry if new URL is more canonical

[ausi]

Fixes #8498, #8460

[aschempp]

Instead of deleting and adding a new record, how about updating the URL so the existing entry is re-used?

[ausi]

Instead of deleting and adding a new record, how about updating the URL so the existing entry is re-used?

A new record is not always created, this depends on the if ($objIndex->numRows) on line 208. So I think deleting the record should be OK, otherwise the logic gets more complicated.

[leofeyer]

That's not entirely true. We could run:

$db
    ->prepare('UPDATE tl_search SET url=? WHERE id=?')
    ->execute($arrSet['url'], $objIndex->id)
;

Then the query in line 201 would find the existing record.

[leofeyer]

If we really want to delete the existing record, we must also remove the child records from tl_search_index.

[ausi]

That's not entirely true. We could run:

$db
    ->prepare('UPDATE tl_search SET url=? WHERE id=?')
    ->execute($arrSet['url'], $objIndex->id)
;

Then the query in line 201 would find the existing record.

I don’t think so. This would result in a Query error: Duplicate entry '...' for key 'url' (UPDATE tl_search... error. See the reproducing steps from #8498 (comment)

[ausi]

If we really want to delete the existing record, we must also remove the child records from tl_search_index.

Addressed in fb40870

[leofeyer]

I don’t think so. This would result in a Query error: Duplicate entry '...' for key 'url' (UPDATE tl_search... error.

You are right, of course.

[aschempp]

Why would that result in a duplicate index issue when we update the URL?

[ausi]

Because an entry with the new URL might already exist and the URL column has an unique index. I tried it out and got the Duplicate entry '...' for key 'url' error...

[aschempp]

I don't understand that.

1. we are searching for a database record by checksum
2. if that record exists, we must delete it and not modify the URL, because the URL could exist
3. then we create a record with that URL?

[fritzmg]

@aschempp see codefog/contao-cookiebar#21 (comment) for an example. The same page could be present multiple times in the tl_search table, with different GET parameters and different checksums. If you try to update the URL for one particular checksum, you might have duplicate URLs.

[aschempp]

I do understand that, but how can this be an issue if we add a new database record with exactly that URL and checksum just a few code lines later?

[ausi]

I do understand that, but how can this be an issue if we add a new database record with exactly that URL and checksum just a few code lines later?

A few lines later there is a check if an entry with that URL already exists. Only if it doesn’t exist a new record is created, otherwise the existing record gets updated. And this record doesn’t have the same URL as the record we have deleted.