Allow CTE on more CTE safe functions#10771
Conversation
TimWolla
left a comment
There was a problem hiding this comment.
Actually “Request changes” to make the concerns more visible to other reviewers: #10771 (comment)
This reverts commit 8e31c3e.
This reverts commit 069fa1a.
This was accidentally added in phpGH-7780, but since it takes a callable argument, this flag is useless on this function. Closes phpGH-10859.
This reverts commit c6c4f21.
|
Okay I went through the whole list in detail now. Two functions need to be removed:
I also see you added an XFAIL in one of the tests, I think it's better to not XFAIL the test. Instead, just disable opcache on that test. That way, we can still catch bugs. Other than the remarks above I think this PR looks good (but someone else should double check). |
|
There are test failures because you forgot the PHP tags in the SKIPIF section. |
ndossche
left a comment
There was a problem hiding this comment.
I think this looks good now. I'll wait a few days to see if more feedback comes in, but if not I'll do do the merge.
As a side note, I think that the better solution for the observers is to simply disable SCCP optimisation for those tests and revert the changes done to them, but I will do that during the merge.
I agree. |
|
Merged manually, thanks. |
No description provided.