'per field' storage engine

Also, support for read-only backends implies :
- hook_storage_info()
- fields (instances) without widgets,
- ... ?

Better title.

We should not feel absolutely constrained by the current incarnation of Views. It does not HAVE to work the way it currently works

True, but we do have serious hints that it will not let you filter/sort on different data sources, because of the theoretical implications. At least, that's how I understand Crell's followup in http://www.palantir.net/blog/remote-data-drupal-museums-and-web-2009#com...
"Searching across separate datasets has no solution that anyone has found that does not involve first caching everything in an intermediary server and then searching that. We're not going to solve that in Fields API in D7" [and I'd add: most likely not in Views D7 either, if Views D7 needs to run on your average shared hosting plan].

But even if Views D7 brilliantly finds a way no one thought of earlier, and lets you do this kind of cool stuff, it will very likely need this information anyway:
- what is the storage backend for each field ?
- what is the primary storage for each Views-enabled entity.
Option 2) above is *just* about making this information available, so I don't think we need to know much more about what views will allow and how, before we can start to move away from 'per site' storage backend.

Obviously, I'd very much like Crell and merlinofchaos to at least subscribe to the issue ;-)

Clarified on IRC, merlinofchaos meant "marking fields *and entities* as belonging to a particular storage engine is, IMO, precisely the way to go".

OK, first stab at 'per field' storage backend.

- introduces hook_field_storage_info(), to let modules expose their storage types - quite similar to field or widget type info.
- introduces corresponding helper _field_info_*() functions.
- introduces $field['storage'] = array('type' => 'storage_backends', 'settings' => array(...));, and modifies field_create_field() so that it adds $field['storage']['type'] = 'field_storage_sql' if not specified.
- invokes all storage modules on object operations : field_attach_[load|insert|update|delete|delete_revision]().
It's the responsibility of storage modules to only act on fields they actually store. Thus, if several fields in the object are stored by the same storage backend, the module can 'group' queries' if it wants / needs.
- updates field_storage_sql.module accordingly.
- while I was in there, I removed hook_field_storage_[create|rename]_bundle(), they are no different than hook_field_attach_[create|rename]_bundle().

Most probably not perfect, + will need additional tests for "load|edit|delete object using different fields from different backends" (which means an another test backend, yummy - will probably be variable_[get|set]() based, shouldn't be too hard...).

Posting this to see where we stand in terms of test failures...

Two exceptions, not so bad :-). Updated patch should fix these.

Still TODO:
- a couple PHPdocs
- add a dummy test storage engine and test that an entity can be loaded / saved with fields using different storages.
- handle the case of 'storage module disabled' (this is another case of 'field inactive', there is no working around 'no storage available')

Tests green, cool, back to CNW.

So when I'm adding a field to a fieldable entity type, do I choose the storage backend like I choose the widget, from a list of storage choices that support that field type?

Yes, that's the idea. Except storage engine is set at field creation time and cannot be changed afterwards (that is, not without the mythical 'migration' contrib module). And storage engines are not limited to a list of field types they support.

The idea behind the "f_a_pre" hooks is that any module can choose to store (or load, or query/filter/sort) any field data based on any criteria it chooses, and then the default storage engine picks up anything that is left. I realized a while ago that the pre-hooks are really part of the storage API, not the attach API. Also, the pre-hooks are kind of awkard, so rotating them into a better API is a fine idea.

But I wonder about tying a particular field to a field storage backend as a field property.

Hm. Complex area.

I don't think I really envisioned the f_a_pre_* hooks as a way to implement storage backends so far, rather 'optimize per-field-table local storage' (pbs) or 'let $node->status be a field while living in the {node} table'. To me those were two layers addressing slightly different needs.

To me one of the key features in 'abstracted storage handling in Field API' has always been "this specific field lives in a foreign, non-local data repository". The discussion above showed the need for "this field lives in the same foreign location that the foreign_object entity type and can Views can use it in filters and sorts on foreign_object listings".

I've always assumed this was more natural on a field-by-field basis, where each field specifies where it wants to be stored. Mentally trying to reconsider since, true, a large part of the 'storage backend' job can be done through f_a_pre_* hooks.

To summarize:
1) current situation: there's only one actual 'storage backend' active for all the site's fields (usually field_sql_storage.module). 3rd party modules can hook in and bypass it behind the scenes to serve and write field values from/to different locations, according to their own internal logic. Alternate ('remote') field storage use those hooks, and thus not 'storage backends' modules (unless they have a vocation to store node bodies, file uploads, etc..., which is doubtful).
2) direction taken in this patch: several storage backends are active at the same time. Each field explicitly specifies in $field['storage'] which one it uses; if none specified, the site's 'default backend' (usually field_sql_storage.module) is used. 3rd party modules can still hook in and tweak (or do messy things) like previously, but field API 'knows' where a field is supposed to live.

Trying to think of what this means :

- field creation workflow with 1) is
a) field_create_field($field). The 'one and only' active storage module creates the corresponding tables.
b) Inform the 'f_a_pre_* hook'-module that this field_id is in the list of fields it should intercept. Meaning each 'alternate storage' module need to provide it's own way of doing this - a UI or some variable-based settings, probably. There's no guaranty that those settings won't change, or be added later, or get out of sync, meaning a field suddenly changes it's storage location (which we're not supposed to support; means existing data is lost and stays stale).
With 2) this is just:
$field['storage']['type'] = 'my_remote_backend'; field_create_field($field);

- Several 'f_a_pre_* hook'-module can coexist, each one deciding to 'act' (bypass, mirror...) or not on the storage of a given field according to its own logic that only lies in its code. In case of 'bypass' (which is what an 'remote storage' would do), which one wins is a matter of hook execution order. I'm afraid this could get messy.

- While the (contrib or custom) 'f_a_pre_* hook'-module is disabled (think major core upgrade), the field switches back to field_sql_storage. There's a large window for bad things to happen until it's re-enabled. With 2), the field becomes 'inactive' (like when the field type itself is unavailable), and stays out of the way.

- With 1), the Views integration for field_sql_storage.module exposes the field as stored in a regular, local table, thus available for a 'sort' of 'filter by' on local entities (nodes, comments...). The 'f_a_pre_* hook'-module should hook_views_data_alter() those entries out, in addition to providing its own views integration. Possibly no big deal, I'm not 100% sure.

- Also, I expect some of those 'foreign repositories' to be read-only. With 2), storage 'descriptors' in hook_field_storage_info() + $field['storage']['type'] make it easy to do 'if storage module is read-only, don't show the field's widget in edit forms' in field_default_form(). Not doable with 1) since the black magic happens behind the scenes inside hook_f_a_pre_*().

- More generally, with 1), we never know where a field is actually stored and how this storage behaves; we just invoke the hooks and let them do their stuff. I suspect that could be a problematic limitation in more places than just edit forms.

How would pbs.module work, in which enabling a new module changes the way some existing fields are stored?

Well, to me the scope of pbs is to provide an alternate storage structure for fields stored by the regular field_sql_storage ? Or at least, fields stored in the local db by an alternate 'other_local_sql_contrib_storage_so_much_more_clever_than_cores_sucky_default_storage' module ?
Then it would act only if ($field['storage']['type'] = 'field_sql_storage') - or, $storage = field_info_storage_type($field['storage']['type']); if ($storage['location'] == 'local_db')

Again, this is a complex area and I'm not claiming I have the answers :-/. As the above shows, I'm currently biased on approach 2), though...

Thought of that one later:
If I want to implement an alternate storage backend,, I'm basically writing an "equivalent" of field_sql_storage.moule, and it's that module I'll naturally study and adapt. Thus, rather hook_field_storage_*() than f_a_pre_*().

Updated patch:
- completes the docs
- implements a dummy but full-fledged test storage backend using the {variables} table. Fun.
- tests saving / loading an object with fields using different storage backends

still TODO
- handle the case of 'storage module disabled' (this is another case of 'field inactive', there is no working around 'no storage available')

- Fixes the exceptions in #15,
- adds handling of 'storage module disabled'
- adds a test for the 'inactive field' behavior on module enable / disable.

Let's see what the bot says, tests are a *pain* to run on my setup tonight.

Fixes the tests for 'active / inactive field'

Indeed. Always my recurring habit of commenting some tests out to run the one I want quicker, and then forgetting to uncomment before rolling the patch.

Yes, it's the responsability of the storage module to act only on fields using the storage backend it implements.
See field_test_field_storage_load(), or

-      if (!isset($skip_fields[$field_name]) && (!isset($options['field_name']) || $options['field_name'] == $instance['field_name'])) {
+      $field = field_info_field($field_name);
+      if ($field['storage']['type'] == 'field_sql_storage' && !isset($skip_fields[$field_name]) && (!isset($options['field_name']) || $options['field_name'] == $instance['field_name'])) {

in field_sql_storage_field_storage_load().

Simple reroll after #372330-17: fields : do a pass of TODOs checking got in.

Attempt to simplify the code pointed in #23-#24. That logic is now moved to the caller (field_attach_load()), hook_field_storage_load() receives a list of $fields it should load, period.
Just converted _load for now, letting the bot run.

Oops, that one should be better.

silly typo.

OK, I'll wait a bit before rerolling, then ;-)

Just a note that I'll try to move forward on this by the end of the week.

Painful reroll after 'Bulk delete', no changes, just checking tests.

Incorrect reroll indeed. field_name / field_id snafu. An error on field load doesn't go unnoticed :-p
Let's see with this one.

Hm, I moved forward on the conversion of field_storage hooks started in #26, but the commit of #367595: Translatable fields will make for another reroll fest.
Well, glad that the post-'Bulk delete' reroll at least returned green for a short period of time :-/

Painful reroll after the Translatable field patch.
Not finished yet, just checking how bad tests are.

heh, reapplying a patch rolled in the drupal_function_exists()-era is bound to cause a few surprises...
Let's try again.

Better. How about this one ?

And this one ?

That one should leave only 'bulk delete' failures. That's it for tonight.

New patch should fix remaining failures. Patch is not finished yet, but at least we're through with the 'translatable fields / bulk delete' reroll hell.

Still TODO:
- update the 'test storage engine' and make sure it passes all tests (I'll probably let the bot run in a dummy issue to not add more clutter to this one)
- update PHPdocs
- figure out a WTF around translatable fields in field_sql_storage_field_storage_write()
- write a summary :-/

re bjaspan #50:
Agreed that fields having separate storage locations makes #569224: Expose field storage details through field attach. a little trickier, but I don't think the patch changes anything on that regard. In current HEAD's 'one single storage backend for all fields' approach, PBS or field_parallel_memcache_storage.module already imply primary + secondary storages and the need for 'exposed storage info' to account for that. This patch only makes it so that the primary storage is not necessarily the same for all fields on the site.

The "pre" hooks probably belong in the field_storage namespace, not the field_attach namespace.

100% agreed, we've been postponing that for a while. Do we want to to the change in this patch, though ?

forgot to set to CNR

reroll after #571934: Omissions in field_delete_{field,instance}()

Should be better. Also updated field_test_storage, but I still need to validate its tests.

Reroll after #569072: Clean field_attach_* namespace and #569240: Remove field ID from field table names.

+ Behind the scenes, making progress on having the test_storage backend pass all tests.

I'm done with the remaining TODOs, patch should be ready for human review now.

Summary of what the patch does :

- Promotes 'storage backend' as a pluggable field property, quite like field types, widgets or formatters
--> introduces hook_field_storage_info(), hook_field_storage_info_alter(), field_info_storage_types(), field_info_storage_settings()
--> {field_config}.storage_type
--> new section in the $field definition array():

$field['storage'] = array(
  'type' => the name of the storage backend
  'settings' => settings for the storage backend (not sure about actual use cases, 
                but this stays consistent with what we have for field types, widgets and formatters)
);

The storage backend should be specified in field_create_field($field). By default, we use the backend specified by the 'field_storage_default' variable. We don't support changing storage backend on existing types (that's a job for contrib)

- Marks fields as 'inactive' when their storage module is disabled (like what we currently do when the field type module is disabled).
--> {field_config}.storage_module, {field_config}.storage_active, field_modules_disabled(), field_associate_fields()
Adds a test for this.
--> FieldCrudTestCase::testActive()

- The patch currently does not add any UI. All custom fields created through field_ui use the default storage backend. I'm not sure we'll want to change that. Using different storage backends is a rather advanced task.

- All hook_field_storage_[load|write|delete|...]() functions receive a $fields parameter, listing the fields they should act on. This notably lets us move some tricky logic out of the hooks and into the caller field_attach_* functions.

- Removes hook_field_storage_[create|rename]_bundle(), storage backends can use the existing hook_field_attach_[create|rename]_bundle() 'generic' hooks. Note that this was already the case for _delete_bundle().

- Fixes a snafu around multilingual fields in field_sql_storage's hook_field_storage_write() ($object->name = empty array did not wipe existing data - added a test for that), and streamlines this code a little.

- Adds a dummy but full-fledged 'field_test_storage' backend in field_test.module (stores field values in the {variable} table)

- Adds a test to check we correctly save and load an object with two fields using different backends.
--> FieldAttachStorageTestCase::testFieldAttachSaveLoadDifferentStorage()

- Sets the bases for reusing field tests across different storage engines. Coding the dummy 'field_test_storage' backend showed that writing an alternate storage backend without a full test suite to validate upon is fairly hard.
--> $default_storage property on the FieldTestCase parent class.

In a followup patch, we should have field.test run against the 'field_test_storage' backend, and have field_sql_storage.test take care of running them with field_sql_storage.
Would IMO add too much cruft in this patch. For now I just upload two versions of the patch (one line change), to check that both 'field_test_storage' and 'field_sql_storage' backends pass all our tests. Only field_multiple_storage-443422-60.patch is a candidate for inclusion.

Try again.

bump :-)

I think this is ready, but we need some reviews so that it can reach RTBC in time...

re #66 : "I did not change the $fields arg to field_storage hooks to have field structures as values. I actually do not see why they aren't/can't be field structures, but that is an easy fix for a followup patch, and is not critical."

I think I went back and forth on this. The thing is the 'load' hook uses $fields as an array mapping field ids to the ids of the objects to load for each field. The logic to collect this info is a bit tedious so it's more precious to provide the 'load' hook with that rather than the field structures, that are easy to access with the id. For the other (non 'load') hooks, I just stayed consistent. I guess they could receive the $field_id => $field array instead of the redundant $field_id => $field_id array. Building and passing that array has a slightly higher memory cost, but would save a few field_info_field_by_id() calls.

re #71: That's how the hooks currently work for field type / widget hooks : [module]_field_load() or [module]_field_widget() are called by module, not by field type or widget type. If a module implememts several field types or widgets, those hooks are likely based on a switch. That's one of the worst things in our current architecture, that we didn't bother / dare change from CCK : we handle as 'hooks' what are in fact callbacks (or methods in a OO design) :-(

And thanks for bringing this home, Barry :-)