convert database drivers to use new static caching API [#422366]

follow-up to: #254491: Standardize static caching

see last patch examples: http://drupal.org/node/254491#comment-1430180 and also: http://drupal.org/node/224333#static_variable_api

Apply this conversion pattern to includes/database to convert all static variables there. Pay close attention to any place a reset parameter is provided and add a call to drupal_static_reset() where appropriate (e.g. in any calling function that uses the reset parameter)

Comment	File	Size	Author
#12	jamesan_422366-12.patch	3.55 KB	JamesAn
#12
#11	jamesan_422366-11.patch	4.47 KB	JamesAn
#11
#10	jamesan_422366-10.patch	4.59 KB	JamesAn
#10
#8	jamesan_422366-8.patch	4.59 KB	JamesAn
#8
#5	jamesan_422366-5.patch	5.15 KB	JamesAn
#5
#2	jamesan_422366-2.patch	5.21 KB	JamesAn
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

pwolanin CreditAttribution: pwolanin commented 3 April 2009 at 13:08

Per discussion w/ catch - if you have a variable name more complex than just __FUNCTION__, use a ':' to separate the suffix (this will avoid colliding with any other valid function name). e.g. :

&drupal_static(__FUNCTION__ . ':second_var');

Comment #2

JamesAn CreditAttribution: JamesAn commented 10 April 2009 at 07:33

Status:

Active

» Needs review

File	Size
jamesan_422366-2.patch	5.21 KB

All the static variables I encountered here didn't seem to ever need resetting.

There were a couple adjustments I made:

I removed a couple of the conditional statements (in pager_get_querystring and theme_pager_link) as they simply initialize the static vars.
The static vars that weren't converted were documented
I made a variable in the pgSQL and SQLite drivers static, mirroring the MySQL driver.

Comment #3

10 April 2009 at 07:45

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #4

Berdir

German

Switzerland

CreditAttribution: Berdir commented 10 April 2009 at 22:22

@#2
You can't use function calls when defining static variables, that's why these additional if's are needed.

Comment #5

JamesAn CreditAttribution: JamesAn commented 12 April 2009 at 04:24

Status:

Needs work

» Needs review

File	Size
jamesan_422366-5.patch	5.15 KB

Oops. I missed that PHP detail.. ^^" That explains why the patch didn't work. Yey me and my self-taught PHP knowledge. XD

Comment #6

pwolanin CreditAttribution: pwolanin commented 12 April 2009 at 13:14

I might also convert function pager_get_querystring() - for example if a test is executing several different paths during one page load.

Comment #7

Dries CreditAttribution: Dries commented 13 April 2009 at 09:42

Status:

Needs review

» Needs work

theme_pager_link() contains some changes that seem irrelevant to this patch, and that are not documented.

Otherwise this looks fine.

Comment #8

JamesAn CreditAttribution: JamesAn commented 13 April 2009 at 23:40

Status:

Needs work

» Needs review

File	Size
jamesan_422366-8.patch	4.59 KB

Thanks for catching those bits. I've converted pager_get_querystring() and fixed the problem with theme_pager_link().

Comment #9

13 April 2009 at 23:50

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #10

JamesAn CreditAttribution: JamesAn commented 14 April 2009 at 02:18

Status:

Needs work

» Needs review

File	Size
jamesan_422366-10.patch	4.59 KB

Oy vey.. there was a dangling parenthesis.

Comment #11

JamesAn CreditAttribution: JamesAn commented 14 April 2009 at 02:25

File	Size
jamesan_422366-11.patch	4.47 KB

I broke up $string = &drupal_static(__FUNCTION__, drupal_query_string_encode($_REQUEST, array_merge(array('q', 'page'), array_keys($_COOKIE)))); so that drupal_query_string_encode() is only run when $string is NULL.

Here it is.

Comment #12

JamesAn CreditAttribution: JamesAn commented 14 April 2009 at 03:35

File	Size
jamesan_422366-12.patch	3.55 KB

Oops! I mistakenly included pager.inc into this issue. Changes to that file belong to #422370: Revisit and convert mail.inc, module.inc, pager.inc to use static caching API. This patch reflects this fix and no longer includes pager.inc changes.

The patch of the other issue reflects this realization too.