[5.5] Operator optional

[emtudo]

Operator optional and fix type "time mysql"

DOC mysql: https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_time
TIME(expr)

Extracts the time part of the time or datetime expression expr and returns it as a string.

This function is unsafe for statement-based replication. A warning is logged if you use this function when binlog_format is set to STATEMENT.

[sisve]

Why was this merged ...

1. without tests,
2. with a new method signature in the middle of the 5.5 release,
3. without changing orWhereTime?

[sisve]

PHP Warning: Declaration of CustomBuilder::whereTime($column, $operator, $value, $boolean = 'and') should be compatible with Builder::whereTime($column, $operator, $value = NULL, $boolean = 'and')

[emtudo]

How could you make the mistake? Because here for me it does not happen.

As for owWhereTime, it was forgetting to change "@param int $ value" as it is the only thing that needed to be changed.

Do you find it easier to reverse the situation instead of actually fixing the problems?

[sisve]

Do you find it easier to reverse the situation instead of actually fixing the problems?

Could you tell me which problems you were fixing? This PR only made the operator optional, if I read the code correct. There are no tests provided or anything that shows which problems existed previously.

The problem my revert fixes is the changes that causes warnings due to changed method signatures in the middle of a 5.5 release. If you really need these changes, redo them against the 5.6/master branch and provide tests.

[emtudo]

Did you read https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_time?

[emtudo]

TIME(expr)

Extracts the time part of the time or datetime expression expr and returns it as a string.

[sisve]

You've done three things in this PR.

1. Made a backward incompatible change by making the operator optional.
2. Changed a docblock comment to change a type hint.
3. Forgot to change the other docblock comment.

Since the first point is a huge problem I argue that the entire PR should be reverted, and you're welcome to submit a new working PR that isn't breaking existing code, that is a working fix for the things you want to change.

[emtudo]

I ask again, if it's breaking because you did not correct the problem instead of going back?

[sisve]

I do not understand your question. It is breaking because you changed a method signature in the middle of the 5.5 lifecycle. It would have been better if this were targetting 5.6 instead. I argue that since this is a breaking change, it should be reverted.

I am correcting the problem, by suggesting a revert of these changes, so the code goes back to the previous state. We can then try again to implement these things in a non-breaking way.

[emtudo]

Ok! As you preferred!

Thanks!

[antonkomarev]

@emtudo just point it to master branch.