[TYPO3-core] RFC #9505: Make the caches in TYPO3 use the new caching framework

[Ernesto Baschny [cron IT]]

Ingo Renner wrote: on 20.10.2008 13:08:

> here's an updated version that takes Masi's suggestions and questions
> into account.
> 
> changes to v3:
> 
> * "TODO check where $expTime is used, the new caching API can't handle
> it, so it can be removed":
> This one is resolved, the TODO is removed. Throughout the core it was
> used with the parameter's default value only, so I removed it. The thing
> is that the new caching frameworks handles the lifetime of a cache entry
>  when saving it, not when retrieving it like the old one did.
> 
> This affected the method t3lib_pageSelect::getHash() and it's identical
> sibling t3lib_BEfunc::getHash().
> For the pageSelect method I found only one place where it wasn't used
> with a value different from the default. This was in tslib_menu, which I
> changed to set the lifetime of a cache entry when saving it instead of
> comparing the lifetime when retrieving the entry.
> 
> * naming of $cacheManager and $TYPO3_CACHE
> these have been changed to $typo3CacheManager and $typo3CacheFactory
> (both are still global)
> Both, the cache manager and the factory now implement the new
> t3lib_Singleton interface
> 
> Hope we can get this one through now.

Still one issue:

You change the SQL-definitions of cache_hash and cache_pagesection:

 CREATE TABLE cache_hash (
-  hash varchar(32) DEFAULT '' NOT NULL,
-  content mediumblob,
-  tstamp int(11) unsigned DEFAULT '0' NOT NULL,
-  ident varchar(20) DEFAULT '' NOT NULL,
-  PRIMARY KEY (hash)
+  id int(11) unsigned NOT NULL auto_increment,
+  identifier varchar(32) DEFAULT '' NOT NULL,
+  crdate int(11) unsigned DEFAULT '0' NOT NULL,
+  content mediumtext,
+  tags mediumtext,
+  lifetime int(11) unsigned DEFAULT '0' NOT NULL,
+  PRIMARY KEY (id),
+  KEY cache_id (identifier)
 ) ENGINE=InnoDB;

and:

CREATE TABLE cache_pagesection (
-  page_id int(11) unsigned DEFAULT '0' NOT NULL,
-  mpvar_hash int(11) unsigned DEFAULT '0' NOT NULL,
-  content blob,
-  tstamp int(11) unsigned DEFAULT '0' NOT NULL,
-  PRIMARY KEY (page_id,mpvar_hash)
+  id int(11) unsigned NOT NULL auto_increment,
+  identifier varchar(32) DEFAULT '' NOT NULL,
+  crdate int(11) unsigned DEFAULT '0' NOT NULL,
+  content mediumtext,
+  tags mediumtext,
+  lifetime int(11) unsigned DEFAULT '0' NOT NULL,
+  PRIMARY KEY (id),
+  KEY cache_id (identifier)
 ) ENGINE=InnoDB;


The might work on a "fresh" TYPO3 install, but on an upgrade, this will
create the following SQL statements (COMPARE in Install Tool):

Add fields:

ALTER TABLE cache_hash ADD id int(11) unsigned NOT NULL auto_increment;
ALTER TABLE cache_hash ADD identifier varchar(32) NOT NULL default '';
ALTER TABLE cache_hash ADD crdate int(11) unsigned NOT NULL default '0';
ALTER TABLE cache_hash ADD tags mediumtext;
ALTER TABLE cache_hash ADD lifetime int(11) unsigned NOT NULL default '0';
ALTER TABLE cache_hash ADD KEY cache_id (identifier);
ALTER TABLE cache_pagesection ADD id int(11) unsigned NOT NULL
auto_increment;
ALTER TABLE cache_pagesection ADD PRIMARY KEY (id);

Changing fields:

ALTER TABLE cache_hash CHANGE content content mediumtext;
Current value: mediumblob
ALTER TABLE cache_hash DROP PRIMARY KEY;
ALTER TABLE cache_hash ADD PRIMARY KEY (id);
ALTER TABLE cache_hash ENGINE=InnoDB;
  Current value: ENGINE=MyISAM


Executing that in that order will never add the "id" field, because of:

mysql> ALTER TABLE cache_hash ADD id int(11) unsigned NOT NULL
auto_increment;
ERROR 1075 (42000): Incorrect table definition; there can be only one
auto column and it must be defined as a key

This is clearly a limitation of our SQL-update-statement-generator, as
the correct order would be:

ALTER TABLE cache_hash DROP PRIMARY KEY;
ALTER TABLE cache_hash ADD id int(11) unsigned NOT NULL auto_increment
PRIMARY KEY;

(auto_increment has to be defined together with "PRIMARY KEY" for a field).


So to overcome that problem, I would like to know if there is a need for
an "auto_increment" field in a caching table? Isn't that unnecessary
overhead for something that is ought to be fast?


Apart from that, do we need to re-use the table names "cache_hash" and
"cache_pagesection" if we are changing their definition completely?
Maybe those should simply be dropped and some fresh new tables with
different names could be created? That would also avoid the problem of
older extensions trying to access "reg1" field in cache_hash table etc.


Other than that, I did some quick tests, and it seems to be working
fine! Thanks for your efford and persistence on that front! ;)


Cheers,
Ernesto