I am wondering if there is any interest in moving storage from php serialized() format to json_encode() format. The advantages to at least allowing this as an option are:

Interested in any and all thoughts on this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

robertwb created an issue. See original summary.

lolandese’s picture

Issue summary: View changes

Ability to have views function level data extracts (i.e., show a specific row/column in Views)

Interested in any?

Yes. Definitely.

thommyboy’s picture

Ability to have views function level data extracts (i.e., show a specific row/column in Views)
oh yes! ;)

lolandese’s picture

A possible starting point is how the stored table data could look like. Table data as an array of objects where each object represents a table row.

CURRENT:
Taking an example of a table with 2 columns and 3 rows the serialized field_table_value (longtext) in Table: field_data_field_table currently looks like (machine name can differ, based on the custom defined field label for your table field):

a:8:{s:8:"cell_0_0";s:23:"First column, first row";s:8:"cell_0_1";s:24:"Second column, first row";s:8:"cell_1_0";s:24:"First column, second row";s:8:"cell_1_1";s:25:"Second column, second row";s:8:"cell_2_0";s:23:"First column, third row";s:8:"cell_2_1";s:24:"Second column, third row";s:7:"rebuild";a:3:{s:10:"count_cols";s:1:"2";s:10:"count_rows";s:1:"3";s:7:"rebuild";s:13:"Rebuild Table";}s:6:"import";a:2:{s:4:"file";s:0:"";s:6:"import";s:10:"Upload CSV";}}

Unserialzed that would be:

array (
  'cell_0_0' => 'First column, first row',
  'cell_0_1' => 'Second column, first row',
  'cell_1_0' => 'First column, second row',
  'cell_1_1' => 'Second column, second row',
  'cell_2_0' => 'First column, third row',
  'cell_2_1' => 'Second column, third row',
  'rebuild' => 
  array (
    'count_cols' => '2',
    'count_rows' => '3',
    'rebuild' => 'Rebuild Table',
  ),
  'import' => 
  array (
    'file' => '',
    'import' => 'Upload CSV',
  ),
)

 
EACH ROW AS AN OBJECT:
This would be the proposal without table headers:

a:3:{s:10:"table_data";a:3:{s:5:"row_0";O:8:"stdClass":2:{s:8:"column_0";s:23:"First column, first row";s:8:"column_1";s:24:"Second column, first row";}s:5:"row_1";O:8:"stdClass":2:{s:8:"column_0";s:24:"First column, second row";s:8:"column_1";s:25:"Second column, second row";}s:5:"row_2";O:8:"stdClass":2:{s:8:"column_0";s:23:"First column, third row";s:8:"column_1";s:24:"Second column, third row";}}s:7:"rebuild";a:3:{s:10:"count_cols";s:1:"2";s:10:"count_rows";s:1:"3";s:7:"rebuild";s:13:"Rebuild Table";}s:6:"import";a:2:{s:4:"file";s:0:"";s:6:"import";s:10:"Upload CSV";}}

Unserialzed that would be:

array (
  'table_data' => 
  array (
    'row_0' => 
    stdClass::__set_state(array(
       'column_0' => 'First column, first row',
       'column_1' => 'Second column, first row',
    )),
    'row_1' => 
    stdClass::__set_state(array(
       'column_0' => 'First column, second row',
       'column_1' => 'Second column, second row',
    )),
    'row_2' => 
    stdClass::__set_state(array(
       'column_0' => 'First column, third row',
       'column_1' => 'Second column, third row',
    )),
  ),
  'rebuild' => 
  array (
    'count_cols' => '2',
    'count_rows' => '3',
    'rebuild' => 'Rebuild Table',
  ),
  'import' => 
  array (
    'file' => '',
    'import' => 'Upload CSV',
  ),
)

 
WITH COLUMN HEADERS:
Making the presumption the first table row holds the column names (properties) to "store" the table field itself as an object instead it could look like:

a:4:{s:10:"table_data";a:2:{s:5:"row_1";O:8:"stdClass":2:{s:39:"First column, first row (column header)";s:32:"First column, second row (value)";s:40:"Second column, first row (column header)";s:33:"Second column, second row (value)";}s:5:"row_2";O:8:"stdClass":2:{s:39:"First column, first row (column header)";s:31:"First column, third row (value)";s:40:"Second column, first row (column header)";s:32:"Second column, third row (value)";}}s:7:"headers";b:1;s:7:"rebuild";a:3:{s:10:"count_cols";s:1:"2";s:10:"count_rows";s:1:"2";s:7:"rebuild";s:13:"Rebuild Table";}s:6:"import";a:2:{s:4:"file";s:0:"";s:6:"import";s:10:"Upload CSV";}}

Unserialzed that would be:

array (
  'table_data' => 
  array (
    'row_1' => 
    stdClass::__set_state(array(
       'First column, first row (column header)' => 'First column, second row (value)',
       'Second column, first row (column header)' => 'Second column, second row (value)',
    )),
    'row_2' => 
    stdClass::__set_state(array(
       'First column, first row (column header)' => 'First column, third row (value)',
       'Second column, first row (column header)' => 'Second column, third row (value)',
    )),
  ),
  'headers' => true,
  'rebuild' => 
  array (
    'count_cols' => '2',
    'count_rows' => '2',
    'rebuild' => 'Rebuild Table',
  ),
  'import' => 
  array (
    'file' => '',
    'import' => 'Upload CSV',
  ),
)

As you see the column header is used to define the property names used in the object (row). This solution probably stores the table in the most useful way. It requires an additional checkbox in the table field settings (the boolean "headers") to turn ON table headers.

ADVANTAGES:
My use case is the rendering of a table of a node as JSON with Views Datasource:

[
  {
    "Title": "Some title",
    "Nid": "123",
    "Table": [
      {
        "Column 1": "Some value",
        "Column 2 ": "Some other value",
      },
      {
        "Column 1": "Some value in the next row",
        "Column 2 ": "Some other value in the next row",
      }
    ]
  }
]

To accomplish that with the current TableField module I had to override a Views template and change the output with some ugly regular expression. With the described data storage that would not be necessary. The output would straightaway look as above.

It would likely open up other possibilities too. As mentioned by others either in the module's Views plugin to output only certain columns and/or rows, or directly through the module's settings for example to have a hidden column based on role.

RESOURCES:

jenlampton’s picture

I think this is a good idea, and I would welcome a patch that changed the data storage from a serialized array to JSON format, as long as it included an upgrade path for existing tablefields - which could be a little sticky.

I don't really like the idea of having each row as an object. We're only dealing with data here, is there any reason not to keep it an array?

I'm about to make a new release of tablefield, but would welcome this in the following one. If you want to start on a patch please work off the latest 2.x-dev branch.

lolandese’s picture

..as long as it included an upgrade path for existing tablefields

Good point. The extra header option should be added manually though per existing table field after conversion.

..is there any reason not to keep it an array?

If one would desire an output other than HTML, it seems most likely they would go with a standard format like JSON in minimal mode. Storing the data that way avoids necessary tweaking of the output.

lolandese’s picture

Status: Active » Needs review
FileSize
17.88 KB

This patch does not yet include an upgrade path but can already be reviewed meanwhile.

You can test with https://simplytest.me/project/tablefield/7.x-2.x?patch[]=https://www.dru.... Just go to ../admin/structure/types/manage/page/fields and add a table field.

BTW the code is also getting shorter:
1 file changed, 90 insertions(+), 116 deletions(-)
:)

lolandese’s picture

Corrected classes like they were before. <th class="row_0 col_0" instead of <th class="row_row_0 col_column_1" .

New test URL:
https://simplytest.me/project/tablefield/7.x-2.x?patch[]=https://www.dru...

lolandese’s picture

Possible approach for the upgrade path :

Two things should be accomplished programmatically in the update hook of the .install file:

  1. The existing implementations of the table field(s) in the manage fields of the content types should be re-saved to make that the default table when creating new content has the right data format.
  2. All consisting nodes that contain a table field should be re-saved to rewrite the table data in the new format to the database.

Steps necessary for #2:

  1. Find the fields that use the tablefield widget.
  2. Find the content types that contain those fields.
  3. Find the nid's of all content of those content types.
  4. Batch process re-save of these nodes limiting it to the concerning fields, see https://api.drupal.org/api/drupal/modules%21node%21node.admin.inc/functi... and http://drupal.stackexchange.com/a/125108.

Once the patch above is reviewed I will work on the upgrade path as described.

EDIT: Instead of processing each entity it is easier to just process the field tables directly.

lolandese’s picture

This patch includes also an upgrade path. Tested locally with table fields attached to nodes, taxonomy terms and users. It will work with any new entity as well.

Test URL:
https://simplytest.me/project/tablefield/7.x-2.x?patch[]=https://www.dru...

SebasL’s picture

FileSize
52.59 KB
85.39 KB
SebasL’s picture

Created new fresh Drupal installation.
Run the patch in the tablefield module folder:

Applied patch tablefield.install cleanly.
Applied patch tablefield.module cleanly.

Run drush updb -y (upgrade path script to rebuild the information already existing in the database):
Performed update: tablefield_update_7003 [ok]

After running the upgrade path, the information in the database is updated to the desired format. See images database_before.png and database_after.png.

Also the table is still displayed as expected in the UI.

TEST OK.

Database before
Database before

SebasL’s picture

Status: Needs review » Reviewed & tested by the community

  • lolandese committed a4b4259 on 7.x-2.x
    Issue #2697105 by lolandese, SebasL: RFC: Enable Table Data Storage as...
lolandese’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

robertwb’s picture

Trying to verify functionality here -- I cannot see that this STORES data as JSON -- i.e., using json_encode() as opposed to serialize(). I am not complaining, since I did not effort on this :), but just want to manage my expectations upon testing. Seems like this is a render only deal -- which is very cool, just different than I was thinking. Oh the data structure revamping is nice too :)

lolandese’s picture

lolandese’s picture

Version: 7.x-2.x-dev » 8.x-2.x-dev
Status: Closed (fixed) » Patch (to be ported)
Parent issue: » #2189203: D8 port (from the existing 7.x-3.x to a new 8.x-3.x)

Mainly this is the one that requires both to bump up the version to 8.x-3.x as it changes the data structure in the database. As a consequence also an upgrade path should be provided.

Additionally or alternatively, we could implement #2883127: Optional Enable Table Data Storage as JSON .