Smart Date Range: Duration of >=16 years throws WSOD

When you upgraded to 4.1.0-rc2, did you run database updates?

@mandclu. Yes. That updated the date fields.

This is explicitly about the duration field, which I believe maxes out at signed mediumint.

Here's the DB table post 4.1.0-rc2 update:

mysql> describe node__field_rights_window;
+---------------------------------+--------------+------+-----+---------+-------+
| Field                           | Type         | Null | Key | Default | Extra |
+---------------------------------+--------------+------+-----+---------+-------+
| bundle                          | varchar(128) | NO   | MUL |         |       |
| deleted                         | tinyint      | NO   | PRI | 0       |       |
| entity_id                       | int unsigned | NO   | PRI | NULL    |       |
| revision_id                     | int unsigned | NO   | MUL | NULL    |       |
| langcode                        | varchar(32)  | NO   | PRI |         |       |
| delta                           | int unsigned | NO   | PRI | NULL    |       |
| field_rights_window_value       | bigint       | YES  | MUL | NULL    |       |
| field_rights_window_end_value   | bigint       | YES  | MUL | NULL    |       |
| field_rights_window_duration    | mediumint    | YES  |     | NULL    |       |
| field_rights_window_rrule       | int          | YES  | MUL | NULL    |       |
| field_rights_window_rrule_index | int          | YES  | MUL | NULL    |       |
| field_rights_window_timezone    | varchar(32)  | YES  |     | NULL    |       |
+---------------------------------+--------------+------+-----+---------+-------+
12 rows in set (0.00 sec)

Note field_rights_window_duration is still a mediumint.

Hmmm I did miss that this was about the duration. I suppose there isn't any value having it signed, since the validation will check that the start is before the date. Why do you need to store events with such large durations?

> Why do you need to store events with such large durations?

In this case, it's genuinely an 18 year date range, just negotiated up from a 14 year date range. The client wants to store the date range for which rights have been granted for a particular media object, and wants to store that date range authoritatively (not fake it and say the rights started today).

I dunno, copyright licensing is weird.

Regardless of the duration, the module should throw an error on validation rather than WSOD on an invalid SQL insertion, regardless of the data being entered into the UI.

I'm tempted to say that for edge cases like this it makes sense to just use separate fields, but in truth this has come up before, and the storage difference is not huge between mediumint and integer, so this is worth consideration. Also, Smart Date uses integers for storing numbers that if anything are likely to be smaller.

From my quick calculations, moving to integer would allow for storage of ~68 years, not quite enough for an average human lifetime, which seems like it should be a goal to accommodate if making this change. Making the column unsigned would double this, making the duration field able to accommodate even exceptionally long lives. I'm not sure if changing from a signed to an unsigned column creates higher risk for existing data, but we can try it.

Something's not quite right about that math. I swear the duration data is in minutes, not seconds. Signed mediumint is 2^23 = 8.38M minutes = 15.9 years, and the system fails on 16 years but works on 14 years.

So a signed integer (2^31 minutes) would be 4085 years.

(If duration isn't minutes but seconds, something else odd is going on)

Honestly, I think it's more important to put in validation on the date range field to prevent a WSOD.

You're correct. Not sure what NyQuil-induced math I was working, but you are correct that even with a signed integer, 4085 years is the maximum duration, which should be sufficient for anything but the rarest of use cases.

That said, I also agree that the validation is important either way, and probably even more so than the field length change. Would it make sense to create a patch in this issue for the validation, and then open another issue for extending the field length? Or vice versa, I don't mind.

I see WSOD, I open a ticket. No judgement on how fast it gets fixed.

Either 1 ticket or two works for me. I see the justification for both, but if they're two separate commits, they'll definitely need to be sequenced.

Just some context - after a bit more investigation my client was trying to enter in a digital rights window of 99 years, apparently quite common in legal contexts.

https://en.wikipedia.org/wiki/99-year_lease

In history, 99 years was initially a placeholder for "forever" but apparently as the century passed, the law got kind of prickly about being literally 99 years and now it's commonplace.

This is in a field where rights windows are quite frequently just a few weeks, but they want to be able to use the field the same way for both instances.

Aditi Saraf → made their first commit to this issue’s fork.

FYI I have created ✨ Add validation checks for start, end, and duration Active to handle the validation checks as a separate issue.

There's now an MR up in ✨ Add validation checks for start, end, and duration Active , but based on the expanded values that will be implemented in this issue and 🐛 node.field_[name] field needs to be updated Needs review . I suppose theoretically we could merge in the constraints now with the smaller values, and then make updating the constraints part of the work in each issue meant to expand column size.

It's worth noting that this issue and 🐛 node.field_[name] field needs to be updated Needs review are blocked by a requirement for a new setting for the SqlContentEntityStorageSchema class proposed by 🐛 Timestamp field items are affected by 2038 bug Needs work , which now will require a larger set of work to get merged in. As such, I have created ✨ Provide a flag to allow updates to stored schema for fields Needs review as a smaller issue to get the new setting adopted, and hopefully merged in sooner.

There is now an MR with code to expand the duration column to a normal integer.

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