DB drivers need to be able to change the configure database form during install
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | install system |
| Category: | task |
| Priority: | normal |
| Assigned: | Dave Reid |
| Status: | needs work |
| Issue tags: | database, database compatibility |
This issue target to promote the use of table prefix during installation, with a suggestion of maximum length not more than 4cc.
The use of table prefix can solve numbers of potential cross database compatibility problem, such as reserved words conflict with ANSI standard (e.g. 'system' and 'role', check the list of reserved word conflicted). It can also enhance the cross system compatibility with other web application, or even benefit for our multiple site installation.
Some web application are now promoting the use of table prefix during installation, e.g. Moodle suggest to use mdl_ as default table prefix, where eGroupWare suggest for egw_. As reference we may also promote our table prefix with drp_ or something else.
It is a good idea for limit table prefix into a reasonable and acceptable length, because not every database have equal ability to support ultra long constraint name as like as MySQL/PostgreSQL. Moodle give a similar discussion about this topic, and suggest developer always keep table prefix <= 4cc:
[$CFG->prefix] will be the prefix to add to all the objects, but to Oracle, see "Naming conventions II for more info (being conservative with our 30cc limit, this prefix shouldn't be longer than 4cc).
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| promote_prefix_install-1229142243.patch | 2.05 KB | Idle | Unable to apply patch promote_prefix_install-1229142243.patch | View details | Re-test |
#1
I generally support this idea, however we need to make sure not to introduce a usability regression.
Uncollapsing the advanced fieldset IMHO is a usability regression. Our aim is to make install as easy as possible and to reduce the clutter not add more to it.
Also,
is tech talk we definitly need to avoid.
It should be enough for the moment if we keep the fieldset collapsed as-is, but mention both aspects in the field description. Also, I prefer "dru_" as the default prefix, but that's definitely a matter of taste. How about this:
I'm aware of "database types" being not quite exact. However "database management systems" is too complicated and "database systems" not more correct.
#2
Minor correction: placeholder no more needed.
#3
I think this caters to a wider problem: there should be a way for database drivers to alter this form. It's already necessary for SQLite and then your DB drivers can take advantage and set dru_ as a default there.
I think this patch only should introduce the form_alter method to DatabaseInstaller and then we can work from there.Another thing that'd be good to patch here is that I do not know why the DatabaseInstaller class is in install.inc, I definitely would move that to includes/database/install.inc. Move, add a method and be done.
#4
Here's my very rough initial stab at introducing a form_alter function to the database installer classes that I'm pretty sure will not install. To do this, it's going to require that the database type and database options screens be split into two different pages.
#5
OMG I have this working and installing now! I'm rather proud of getting this to work with install.php...what a PITA! Some parts of the code are a little hackish, but I have attached screenshots to demonstrate the new install process and the difference between the MySQL configure screen and the SQLite configure screen.
EDIT: I forgot to expand the 'Advanced options' in the sqlite install screen, but there's only the table prefix field there (no host or port fields). Ignore the special format thingys around the username and password fields. It's my LastPass extension at work.
#6
#7
The last submitted patch failed testing.
#8
I don't think the PIFR will ever like this code since it messes with the install process.
#9
<?
+ $form['basic_options']['database']['#description'] = st('Enter a path to a sqlite file. The database will be automatically created for you.');
?>
These are unclear and SQLite needs to be capitalized properly