schema not correctly rebuilt [#199654]

While debugging CCK HEAD and trying to figure out why certain field settings would not 'take', I finally found out the default to be in drupal schema building. When refreshing, we don't start from scratch but build on the statically cached $schema. Plus the order of array_merge args ensures new entries don't override previous ones.

Provide 'how-to-reproduce' steps is not easy.
I'll be bold and promote this to critical - at least it is for CCK :-)

Comment	File	Size	Author
	schema_rebuild.patch	909 bytes	yched

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

yched CreditAttribution: yched commented 10 December 2007 at 21:39

'I finally found out the default to be...' => 'I finally found out the problem to be' (french faux-ami, heh)

Comment #2

KarenS CreditAttribution: KarenS commented 11 December 2007 at 19:30

Status:

Needs review

» Reviewed & tested by the community

I also was seeing erratic results where sometimes the schema seemed to be 'stuck' on the old values when I tried to refresh it, and this may explain the problems I was seeing.

It seems clear than anytime some asks for the schema to be refreshed, the new values must override any that previously existed. And it seems equally clear that if you are asking for the schema to be refreshed we should *not* start with the cached value but with a new array.

I don't know of any use cases (yet) outside of CCK, but any module that tries to force a refresh of the schema will get bit by this bug.

I should also point out that this patch fixed a bug in CCK at http://drupal.org/node/198931.

Comment #3

yched CreditAttribution: yched commented 11 December 2007 at 19:47

Actually, the bug about stale values staying after the refresh is fixed by the

+      $schema = array();

line. Rather straightforward, was probably simply overlooked when writing the original function.

The other part

-        $schema = array_merge($current, $schema);
+        $schema = array_merge($schema, $current);

is more about enforcing the order of precedence : results from later called hooks override previous results. I *think* this is more inline with other hook invocations in core, but should have no real impact here (no modules should claim the same tables - if they do, then the site probably has a larger conflict problem anyway).