[#20561] - [4.0] Normalize finder tables and add default values
- Fixed in Code Base
- 27 Apr 2019
- Medium
- Build: 4.0-dev
- # 20561
- Diff
- Hackwar:j4finder_database
- Success Hound No violations found. Woof! Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Failure continuous-integration/drone/pr the build failed Details
User tests: Successful: Unsuccessful:
This is a new version of #13517 which originally was supposed to fix #13509. Since all of my finder work is aimed at 4.0, this PR is, too. I also extended this further and since MSSQL is not supported in 4.0 anymore, I've not adopted those changes.
Since this PR is now open for nearly a year, I also reworked this PR to incorporate all DB changes to the finder tables in one big update file instead of several smaller ones.
Changes
- Changed all field types to be identical across DB systems and across tables
- Added default values for all relevant fields
- Added indexes where appropriate
- Updated component SQL files
- Merged all DB changes for 4.0 for Finder into one update file
Considering all the changes that happened to finder, a complete re-indexing is necessary. This requirement is not related to this PR, but comes from the removal of the sharding, the new taxonomies and several other changes. This also means that the filters will have to be manually recreated. All these updates can be done on filled tables, but that would make little sense, since we have to run a complete indexing run anyway, thus this PR first clears the tables and then does the updates. That should help with timeout issues for large indexes.
How to test
I'm quoting the original dev of this PR:
- SQL code review
- Test update: Apply patch, manually run the sql query for your Db system, confirm db table, confirm all fine
- Test install: apply patch, delete configuration.php do a normal install, confirm db table, confirm all fine
@alikon Can you check the Postgres code? Is there a difference between VARCHAR and CHAR in Postgres? Maybe we want to change the language columns from VARCHAR to CHAR? I'm confused because the original author did not change those.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | SQL Administration com_admin Postgresql com_finder Installation |
| Labels |
Added:
?
|
||
Do we need to remove the default value for postgres, too? I couldn't find anything if the text field in postgres has a default value or not. SQL standards. A joy for everyone...
I found https://www.postgresql.org/docs/9.3/static/ddl-default.html
If no default value is declared explicitly, the default value is the null value.
IIRC default value can be set for text type in postgresql,
but it would be good to have the same default value in both databases.
Updated the stuff according to your request
Why does this component have its own install.sql ??
Other components do not
These SQL files are (most likely) still from the origin of this component. Yes, we don't do this elsewhere. On the other hand, I actually like that we have this component in a state where you could just zip it up and install it into a site where it is missing. Maybe we should strive for that for our other components, too.
This branch is out-of-date. Can I still test the sql changes or should I wait?
The out-of-date message can mostly be ignored, unless there is an additional message that there are conflicts. In any case I updated this PR to the latest changes.
I have test the number 2 - update an installation!
I installed the update sql file and played a lot with the smart search. I could not detect any issues. Everything works as before.
When I figure out how I can put a patch into my system with git, then I'll test number 3 - new installation. ;)
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20561.
If you have your own joomla on git then you can apply this patch by command like (on linux):
curl https://patch-diff.githubusercontent.com/raw/joomla/joomla-cms/pull/20561.diff | git applyor simply download the patch from the above link and enter git apply 20561.diff
I've adopted the requested changes. Please review this for merging. Thx.
I've rewritten this PR to also combine all DB changes for 4.0 of Finder into one file and consolidate the different changes. This one file should update all the table at once. I'm considering also adding truncate statements for all tables, since it is pretty useless to alter these tables, adding indexes and all that, when they are full of data and take ages to process, while they also all have to be deleted and re-indexed anyway.
I updated the initial description and did some last changes. Please advise how to progress with this.
| Labels |
Removed:
J4 Issue
|
||
I changed all the requested things...
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-04-27 16:12:06 |
| Closed_By | ⇒ | wilsonge |
LGTM. Thanks!
@Hackwar @wilsonge If this requires reindexing anyway, wouldn't it have been a good time to change the collation of the finder tables to utf8mb4_unicode_ci?
The only reason why we kept some finder tables with utf8mb4_general_ci was that we did not want to force a complete reindexing when doing the utf8mb4 conversion. But that mix of collations has caused some issues from time to time, e.g. when other 3rd oparty extensions doing joins to finder and other core tables.
If updating or migrating from 3.10 to 4.0 will anyway require a complete reindexing of finder, we can change collation in 4.0.
you are right. Would you create a PR to do that?
With the changes in #20185 merged into this branch and conflicts fixed, this is ready to merge.?