database error in AddRecordSchedule returns 'ok'

[Jeff Dwork <jeffdwork47@…>]

A POST to AddRecordSchedule? with null StartTime? causes a database error but returns 'ok' to poster:

snip from post:

 &StartTime=&EndTime=&

snip from log:

#012Driver error was [2/1048]:#012QMYSQL3: Unable to execute statement#012Database error was:#012Column 'starttime' cannot be null
Jun 14 23:12:25 vcr2 mythbackend: mythbackend[1185]: I HttpServer51 httprequest.cpp:368 (SendResponse) HTTPRequest::SendResponse(xml/html) () :200 OK -> 127.0.0.1: 8

[Bill Meek]

Please do: mythbackend --setverbose http and then attach the output from the POST. It should look similar to the 1st attachment. Should be easy to test (for me) but I'm getting a 500 error and proper logging of the empty times. The tests in libs/libmythtv/recordingrule.cpp haven't been touched since 2013.

[Roger Siddons]

For search rules , eg PowerSearch?, RecordingRule::IsValid? doesn't validate the times (because they're N/A) but the Save still tries to write them to the Db.

[Bill Meek]

Roger, I see it, thanks.

Jeff, in addition to the log requested above, please add the actual response from the backend (not just the 200/OK.)

When I tested as a Title Search, I see the DB Error, but the response is -1 (or actually: {"uint": "4294967295"}.)

[Jeff Dwork <jeffdwork47@…>]

log of complete transaction

[Jeff Dwork <jeffdwork47@…>]

I've attached the section of the log showing the complete transaction.

[gigem]

Replying to rsiddons:

For search rules , eg PowerSearch?, RecordingRule::IsValid? doesn't validate the times (because they're N/A) but the Save still tries to write them to the Db.

The schema should probably allow NULLs is such cases. That's something we generally haven't done, though. I also wonder how far such a change would ripple.

[Bill Meek]

Just looking at rules created from the frontend (e.g. Power Search, Record One) and they include a valid start/end/chanid/station in the DB.

Would removing the !isSearch test here: ​https://code.mythtv.org/cgit/mythtv/tree/mythtv/libs/libmythtv/recordingrule.cpp#n940 be valid?

[gigem]

I assume you mean leaving the start/end, duration, chanid and station checks, then? I'd prefer trying the allow NULL solution first before requiring the user to provide valid values to irrelevant columns.

[Bill Meek]

Replying to gigem:

I assume you mean leaving the start/end, duration, chanid and station checks, then? I'd prefer trying the allow NULL solution first before requiring the user to provide valid values to irrelevant columns.

Yes, I meant leaving the tests, making them always required.

If I allow NULLs, then wouldn't that be a "master only" solution because it would be a schema change?

ALTER TABLE record MODIFY COLUMN startdate DATE DEFAULT NULL
ALTER TABLE record MODIFY COLUMN enddate DATE DEFAULT NULL
ALTER TABLE record MODIFY COLUMN starttime TIME DEFAULT NULL
ALTER TABLE record MODIFY COLUMN endtime TIME DEFAULT NULL

And, did you want to convert all k(OneRecord,AllRecord,DailyRecord, WeeklyRecord) existing entries to NULL for the 4 columns above?

UPDATE record SET starttime=NULL, endtime=NULL WHERE search IN (2, 4, 5, 6)
UPDATE record SET startdate=NULL, enddate=NULL WHERE searchtype IN (2, 4, 5, 6)

[gigem]

The conversion of old rules probably isn't necessary. I don't think it would hurt anything, but I don't think it would help anything either.

[Bill Meek <bmeek@…>]

In a29633136dcffff10b74d877fcac2cf14bc863d5/mythtv:

Services API: Add/UpdateRecordSchedule?, prevent DB Errors when timestamps aren't passed

Because they aren't required for SearchTypes? other than None and Manual,
the start/end date/time, weren't being validated. If StartTime? and EndTime?
prarmeters weren't passed, then when an INSERT/UPDATE was done, the NULL
fields caused a DB Error and the API returned -1 ({"uint": "4294967295"}.)

This fix allows NULLs in the start/end date/time in the record table.

Fixes #13056

[Jeff Dwork <jeffdwork47@…>]

I agree that allowing NULLS in these fields is a good thing and will prevent this error, but I think there is still a bug. A database error is not being returned to the initiator. This fix prevents a particular database error, but there might be others in the future.