fix: wrap WHERE conditions in parentheses when soft-delete AND WhereOr are combined#1347
fix: wrap WHERE conditions in parentheses when soft-delete AND WhereOr are combined#1347CyJaySong wants to merge 1 commit into
Conversation
…r are combined (uptrace#1321) When using WhereOr() with a soft-delete model, the AND deleted_at IS NULL condition was only binding to the last OR branch due to SQL operator precedence, allowing soft-deleted records to be returned through other OR branches. Fix: track whether any WHERE condition uses OR separator via a whereHasOr flag on whereBaseQuery. When soft-delete or whereFields conditions need to be ANDed, wrap user WHERE conditions in parentheses if OR is present. Before: WHERE (cond1) OR (cond2) AND deleted_at IS NULL After: WHERE ((cond1) OR (cond2)) AND deleted_at IS NULL Also save/restore the whereHasOr flag in WhereGroup to prevent group-internal OR conditions from incorrectly triggering the outer wrapping.
|
@Aoang Please review it |
I'm sorry for the delayed response. Please understand that the maintainers of this library are not working on it full-time. I did see this PR during the initial round of review, but for various reasons I didn't provide feedback at the time. One of the most challenging aspects of maintaining an open-source library is that we design it based on the scenarios we anticipate, while trying to support as many use cases as possible — yet we can never know every user's specific situation. There's no doubt that this is a bug and it should be fixed. However, the fix involves a breaking change. After users update the dependency, they won't receive any obvious warning or error, which makes it quite risky. I believe the other maintainers are also carefully evaluating the scope and impact of this PR, and considering whether there's a better, less disruptive solution. This kind of assessment takes time, and sometimes a bit of luck too, as it significantly reduces the time available for deep consideration. Thank you for your efforts, and please be patient with us. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. If there is no update within the next 7 days, this pr will be closed. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
|
refresh |
|
I will add my review later this week @Aoang |
for #1321
When using WhereOr() with a soft-delete model, the AND deleted_at IS NULL condition was only binding to the last OR branch due to SQL operator precedence, allowing soft-deleted records to be returned through other OR branches.
Fix: track whether any WHERE condition uses OR separator via a whereHasOr flag on whereBaseQuery. When soft-delete or whereFields conditions need to be ANDed, wrap user WHERE conditions in parentheses if OR is present.
Before: WHERE (cond1) OR (cond2) AND deleted_at IS NULL
After: WHERE ((cond1) OR (cond2)) AND deleted_at IS NULL
Also save/restore the whereHasOr flag in WhereGroup to prevent group-internal OR conditions from incorrectly triggering the outer wrapping.