Storage of empty component values wasteful

I tried this. One complication is that when you edit a submission -- either a draft or a completed submission -- missing component values will be re-defaulted using the component's default. This is because webform_client_form loads the submission, then adds and stored values from the form state, then adds and current values from the form state. If all of this is still a missing value, the form API will set the default.

It would be possible to avoid re-defaulting when editing a finished submission, but to properly handle a draft, we would have to store what was the last page that was recorded in the submission (which may or may not be the last page that was validated, due to the use of the Previous Page button). Store this WOULD give us the option to allow a webform to be resumed on the next page that hasn't been submitted, a feature that has been requested.

So, in short, I would support a patch that does this, but there needs to be some pretty careful consideration about the ramifications.

Adding request to return to last page of draft, the data for which would be needed to implement this feature:
#2278177: Return to the last page when restarting a new draft

Considering this a bit more. One alternative would be to default the submission when it is first created, rather than relying on the form API to do so. In this way, any default value would be set in the submission, and any empty default could safely be not stored without wondering if it should actually be the default value.

The downside is that this precludes the use of submission tokens as defaults. We don't currently support this (I think), but it would be possible to use tokens from previous pages to default subsequent pages.

Current behavior
An empty component stores an empty string in the database. This means that when a form is first presented to the user, the defaults for that page are generated out of the components. When that page is stored, either as a completed submission or as a draft (manual, inter-page, or validation error), those empty values are stored and the default will never again be generated for those components.

If a new component is added, it's value is missing from the database, and so new defaults will be generated should the form be edited again (either as a draft or completed submission).

Choices for implementation

Proposal
An empty component will store nothing in the database. When a page is first presented to the user, the defaults will be generated. When that page is saved as a valid page (either as a draft or as a completed submission), defaults will no longer be generated, and any empty components will remain empty on those pages. Subsequent pages that have not yet been validly saved will continue to receive their defaults.

A slight variation of this is that if a sure returns to a previous page and saves it as a draft (i.e. not validated), then that page becomes the last saved valid page, and subsequent pages will once again get their defaults back if the component is empty. An edge case for sure, probably not noticed by anyone.

Completed submissions will not receive any defaults; any empty components will stay empty. This will be true, even if new components with defaults are created.

Any negative user experience consequences are mitigated strongly because it is very unusual to specify a default and have the user want to enter nothing. Usually when a default is specified, either that value or some other value is desired by the submitter.

This will be implemented by using the "last valid draft page" that is stored as part of #2278177: Return to the last page when restarting a new draft. The hook_update_N can run quickly because there is an index on data. My guess is that deleting 100,000's of rows with emtpy data will happen within the 30 second PHP timeout.

Another side benefit is that our views support can be written to look for not-present data, rather than not-present-or-empty data.

Alternative 1
It would be possible to store empty strings only for drafts, removing them once the submission is complete. This would retain the current behavior for defaulting for drafts, but not for completed submissions. The hook_update_N will be much longer because db_delete doesn't support join and it query will have to be written with an IN or EXISTS where clause. Further, views support cannot be simplified.

Alternative 2
We could add a serialized field to the webform_submissions table and store a list of fields which should have empty defaults. This would then be merged into the input values in webform_client_form. The database would have no empty string values. The hook_update_N would be very time consuming, perhaps needs a batch operation. On a large site, I would think it could take hours.

Choices for how to deploy the implementation:

Proposed deployment
Implement in 7.x-4.x. The edge cases would be assumed to be minor enough to do this with just a warning in the release notes.
Alternative delployments

1. Implement in 7.x-4.x, but allow a settings.php $conf[] to disable the conversion.
2. Defer to 7.x-5.x. If the schema is enhance to allow better storage for multi-checkbox grid and "component collections", this could be bundled with that. I'm not sold on making a 5.x because I fear that user will have no idea which associate modules are compatible, and we might splinter webform support even further.
3. Defer to 8.x

Comments?

@fenstrat -- I my particular case, almost every question is required, so I have a lot of "-1|N/A" options stored. I want to force users to say "Yes, I read and considered the question, but I don't have enough information to answer it". <10% of my rows in the submission table are empty.

But in other cases, there may be a great deal of components with empty items. Remember that every single conditionally-hidden component is an empty value. So if you have a form with a select in which they answer 1 of 5 different sections of the questionaire, 4 of those are going to be empty.

A basic contact form with questions about what they want will probably have lots of empty components.

I'd wager that having 50% or more empty rows is not uncommon. Nate could run a quick query on webform.com, which would be interesting.

if an administrator edits a submission and accidentally modifies a value by repopulating the default.

Thanks Nate. Great to have the background. Excellent point about checkboxes (and other select component variants), but I think the "Proposal" covers this.

During draft creation:
- If the page is <= highest_valid_page, then no additional defaults are added to the submission.
- Otherwise, the default from the component is added as the lowest-priority source (after the submission and user input).
During completed submission editing:
- No additional defaults are added, ever.

The defaults would be prevented by supply an empty string to each component when the default is not desired, and NULL (or nothing) when it is. This will provide compatibility with previous version. In all cases, empty and NULL values are not written to the database.

So it would only be a user who save a draft, clicks previous page, and then save the draft again. If they quit and resume later, when the click Next page, that page will again get defaults (including re-checking any checkboxes). OTOH, that's the default, and they went back, so it isn't totally unreasonable for them to get defaults again.

If this was thought to be a problem that needs addressing, we could store the highest visited draft page, but I'm not recommending that.

The only other difference with previous behavior would be editing a submission (or draft), after the administrator has added a new component. I would argue that what we do now is worse. The submission gains an unexpected defaulted value. Under the proposal, clicking "Save" on an existing submission that had not been edited would do nothing expect update the modified timestamp.

For this relatively small amount of work, we get smaller tables and simpler views. The analysis may be simpler and/or more accurate.

Dammit. A problem

User completes page 1, which validates, and clicks Next Page.
At this point, highest_valid_page is 1.
User then does one of two things:
a) Abandons the page
b) Unchecks a bunch of defaulted-as-checked checkboxes, clicks Save as Draft (or autosaves with failed validation), then abandons the page.

When the user returns to the form:
a) In this case, the defaults should be set from the component definition.
b) In this case, they should not.

I see three solutions:
1) Store the highest saved draft page.
2) See if there is data on the page and if so, don't use the defaults from the component definition. This would be in addition to the page <= highest_valid_page check.
3) Give up.

Option 1) stores another 2 bytes per submission.
Option 2) isn't as bad as it would appear because we are already executing conditionals. It could take note of which pages have data. It would not help if the user removes every default, but that is a pretty unlikely case. Still, it could happen and cause confusion.

A variant on 1) would be to move highest_valid_page and highest_defaulted_page to another table. Actually is_draft could go there too, but maybe only for D8.

I've come to the conclusion this is just too big a change for a .x release. I'm bumping this to 8.x, although if there is ever a 7.x-5.x, it could be done then.

This patch is not complete. Remaining tasks:

1. Add a 'last_draft_page' smallint column to the webform_submissions table.
2. Where last_valid_page is used to determine whether to suppress the defaults from the component definition, use last_draft_page instead.
3. Set last_draft_page in the submit handler.
4. For extra credit, the 'is_draft" column could be replaced by last_draft_page > 0 (i.e. boolean truthy).
5. Every instance of webform_submission_data should be searched for and looked at.
6. All the component analysis hooks should be re-written.
7. The analysis API signature should be changed. They should be passed the number of submissions, so this can be used in the components calculation without re-counting them for each component.
8. Many of the analysis functions loop through every submission counting them in PHP when a simply countQuery() could do the same in the database. This could be extracted into a "core" analysis routine that would be used for several of the components.
9. The e-mail analysis component counts words. Probably a copy-paste holdover from the textfield component.
10. A change record for the data storage should be written. It will certainly affect the analysis of any non-core components.

@Liam -- Re you first point, it is true that components and conditionals can be changed while a draft is in progress, but that's also true of submitted forms. I think it is outside our solution space to address this.

The concept in this patch would be better than what we have today. If a component is edited (say with a default), the mere EDIT/SAVE of a submission changes it today. That would not be true with the ideas in this patch.

To be clear, I think we should do this, but not in 7.x-4.x. Between this and what I'd like to do for schema (supporting field collections with a "collection delta" and grids / multi-select grids with a "sub-component-key"), I'm half tempted to consider a 7.x-5.x as a stepping stone to 8.x.

My fear is add-on module support. There are so many modules that haven't been updated to 4.x that doing a 5.x might be hopeless. That said, I would think that relatively few add-on modules would be affected by these database changes. But views certainly would be.

Maybe we should mark this issue as postponed, since I don't think there is anything 8.x should do now, especially since the storage / fieldable issue is still open.

Or maybe 8.5?

There are some fundamental changes that I'd like to see that break API and database backward compatibility. Now I suppose if 8.5 follows so quickly on 8.4 that there aren't yet any add-on modules to be affected, maybe that's ok.

The changes are of two general types:

1) The format of webform_submissions_data is strongly geared toward the current crop of components. Grids are implemented as a hack, storing their question key in the "no" column, which was intended for multi-valued components like selects. Similarly, there is no easy was to implement field collections. There is also the issue of storing empty values as empty strings, rather than not at all. I'm also thinking that a denormalized datatype might be a wise column, so that views can know how to deal with the data. Currently we have strings, numbers as strings (but we don't know it and have to perform table scans to know), date+times that are formatted as strings that sort, and keys (but we don't know it, so they don't sort in the right order). There might be others that would be worthwhile.

2) Excess hooks, particularly when retrieving data or limiting access. These make it hard to efficiently access the data, especially in the aggregate (views, analysis, export). I'd rather see the data modified before it's stored and be "right" in the database.

Re 8.x development -- your crystal ball is clearer than mine. I remember D7 being very, very, very slow to take off. It was a couple of years before you could really build a D7 site that used a reasonable collection of popular modules. Even today, there are modules I use that still don't have a stable release.