jsedding commented on PR #3054: URL: https://github.com/apache/jackrabbit-oak/pull/3054#issuecomment-5356423209
@reschke alerted me to the following risk. Fixing this properly would require more changes, for which I currently don't have the time. I'll therefore abandon this PR. I think that `PropertyExistenceImpl.getPropertyExistenceConditions()` would need to return two instances if the propertyType is restricted: the one restricted by `propertyType` and one with `propertyType=UNDEFINED`. Otherwise a valid existence condition could be dropped due to an `equals()` mismatch. **What Changed:** `PropertyExistenceImpl.hashCode()` and `equals() `now delegate to `PropertyValueImpl.hashCode()/equals()`, which includes `propertyType` and `selector` in the comparison. Previously, equality was based only on `selectorName` and `normalizePropertyName(propertyName)`. **Why It Is Risky:** `OrImpl.getPropertyExistenceConditions()` uses `Set.retainAll()` to find common property existence conditions across OR branches. If one branch uses `property([test], 'String') = 'x' `and another uses `[test] = 'y'`, the derived PropertyExistenceImpl objects will have different propertyType values (STRING vs UNDEFINED) and will NOT be considered equal. Previously, both would have produced equal `PropertyExistenceImpl` objects (same selector, same property name), allowing the optimizer to derive a `[test] IS NOT NULL` restriction for index selection. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
