ability to clean WITH clause matching CONTAIN process #17563
Conversation
|
Ideally this should come with a test case. |
| if ($cte) { | ||
| $this->_parts['with'][] = $cte; | ||
| } |
There was a problem hiding this comment.
Not sure if a no-op like with(null) makes sense, but other query builder methods with override support allow similar no-ops.
|
Technically this would need to go into 6.x, as the signature change is not backwards compatible. If it's considered low risk, then maybe it could go into 5.next. |
Why isn't it though? All call sites that were valid before are still valid. The addition is new semantic meaning to a newly supported type. We don't have a clear example of 'adding nullability' in our backwards compatibility guide but perhaps it is time we decide on that. I agree on this going to 5.next though as it isn't a bugfix. |
Currently the test is failing, but perhaps I don't understand the use case well enough.
|
I've added a test, and I'm not sure I understand the workflow this change would enable. Any examples you could share? |
|
sorry, been I'll. yes, I have an example that prompted this tweak. I'll drop that here in the morning. |
Now that we have actually typed arguments, adding a type ( |
|
here is an example for where/why resetting the CTE was useful. Existing paginator has memory issues with large MSSQL datasets. |
|
@markstory |
Not really. However, it is currently targetting |
|
Should we change this to support an array of cte instead of a nullable cte? Would that align more with other array parts? |
Given that the |
|
replaced by #18706 |
6 participants
Added NULL condition to overwrite and clear SQL WITH clause matching the CONTAIN process.