as like as cache.inc and cache-install.inc, i would also hope to make variable_get/set function during installation. is it possible?

this is all because Oracle/DB2 database driver will try to write some trimming information into table variable (e.g. according to max. 30 character limitation in column/table/constraint naming under oracle), for forward and reverse mapping. it is now implemented within database driver internally, which will greatly reduce overall performance.

some pre-requirement of the feature request: variable_get/set should be function as normal, during installation. this means that variable_set() should first cache information before table exists; than write save those cached value into table after it is existed. before table exists, variable_get() should also function as normal.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hswong3i’s picture

Title: possible to make variable_get/set function during installation? » make variable_get/set function during installation
Status: Active » Needs review
FileSize
5.78 KB

just a very simple implementation that split variable_get/set/del away from bootstrap.inc, and include required version based on requirement. for installation stage, we will include variable-install.inc for special handling; after that, we will reuse normal version for better performance.

i guess we can do it in some better way, hope someone may give me a hand about this :)

P.S. as i am going to add more cache/variable handling within Oracle and DB2 driver, this issue can greatly improve their performance :)

hswong3i’s picture

bug fix and feature add version: don't do any INSERT/UPDATE for individual variable_get/set/del, but handle overall update by register_shutdown_function('_variable_install_shutdown');. tested with MySQL and seems all fine :)

P.S. little drawback: as variables are not write into table when calling variable_get/set/del, but delay until session close, it seems to be concurrent problem if we have duplicate call to variable_get/set/del from another session... BTW, we are using variable-install.inc during install only, seems this should never happened :)

hswong3i’s picture

hope to detail more information about the needs of this patch.

first of all, most database come with very harsh maximum characters limitation with table/column/constraint/index naming, e.g.

  1. Oracle: all max. 30
  2. DB2: max. 18 (constraint); max. 30 (column); max. 128 (other elements) ...
  3. MSSQL: max. 128 ...

this will become a critical problem for CCK and Views, or other modules which will dynamically generate table schema, as they usually use combine keys as naming format. on the other hand, this will even become a problem for core modules, since we are using combine keys as constraint/index naming, too.

my solution is very simple: create a mapping table for long-to-short and short-to-long mapping. the procedures includes:

  1. use preg_replace_callback() to find out all query elements which conflict with database specific limitation
  2. trim these elements, and setup a mapping for it. e.g. in case of Oracle, "ix_node_comment_statistics_node_comment_timestamp" will translate as "ix_node_comment_statistics_no2" (the last number will be a non-repeat counter value)
  3. save forward/reserve mapping by calling variable_get/set, and reuse the mapping within db_query, db_fetch_array, etc

by using this method, we can simply ignore database specific limitation: database driver will handle this mapping internally. we trade some performance (e.g. setup and use mapping), but grain a better compatibility.

BTW, the conflict is very simple: most mapping will set up during installation (during table creation) by using variable_get/set, but what will happen if table "variable" is not yet ready? variable_get/set don't expect to due with such case, therefore it will:

  1. generate error message (try to INSERT something into table, where target table is not yet exists)
  2. the mapping will lose (variable_set assume always successful for data update: it will not re-insert data if face error, and so it will lose)

this patch introduce an ability to variable_get/set, which cache information before table "variable" exists, and write all cached information into database before session close. for installation, this session will be closed when all tables are successfully created, which means table "variable" is already existed. on the other hand, variable_get/set (special version for installation) will still functioning as like as normal.

hswong3i’s picture

approach by an opposite direction: if we only need table "variable" for storing information used for overcoming database specific limitation, so why don't we create this table before all other tables? the patch is now trying to solve the problem by this method. it is tested with Oracle, and prove as functioning :)

BTW, some conflict can never be solved: length of table prefix is still limited by length of "variable" and its primary key (e.g. pk_variable in case of Oracle). this is because we can never use a mapping to serve the name of "variable", if information is storing within it. anyway, this is just similar as case before, so just simply forget it :)

hswong3i’s picture

Title: make variable_get/set function during installation » first create {variable} table during installation
Component: base system » database system

this patch just simply change the table creation sequence during installation: first create {variable} table before others, and so variable_get() and variable_set() will able to function though the rest of installation. Oracle/DB2/MSSQL driver will make use of this change, in order to overcome size of constraint naming limitation.

patch in #4 can still apply to CVS HEAD. It is tested with both MySQL/Oracle/DB2 and all passed :)

bdragon’s picture

FileSize
1.61 KB

Patch needed rerolling. Also I changed the comment a little.

hswong3i’s picture

Status: Needs review » Reviewed & tested by the community

i guess this patch is simple enough and able to be commit with no question. it should be RTBC

hswong3i’s picture

patch via latest CVS HEAD

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

OK, let it be. Committed.

Anonymous’s picture

Status: Fixed » Closed (fixed)