[SERVER-71525] Rewrite or eliminate tests relying on "failOnPoisonedFieldLookup" Created: 21/Nov/22 Updated: 29/Oct/23 Resolved: 10/Feb/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 7.0.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | David Storch | Assignee: | Projjal Chanda |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | neweng | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Query Execution
|
| Backwards Compatibility: | Fully Compatible |
| Sprint: | QE 2023-02-20 |
| Participants: |
| Description |
|
The failOnPoisonedFieldLookup fail point, when enabled, causes an exception to be thrown when a field named "POISON" is looked up in a document in the SBE VM. There are only two integration tests which make use of this fail point: Both of these tests are about short-circuiting. They are trying to show that if short-circuiting takes place and skips an expression that depends on "POISON". The problem is that this is somewhat dependent on the internals of the implementation. There's nothing to prevent the system from choosing to extract the "POISON" dependency earlier, and reuse a slot containing the resulting value later. In fact, it may be beneficial to do so for performance. Indeed, andrew.paroski@mongodb.com had to inject some special logic when working on I propose that we wiggle out of the current situation by using this ticket to either delete the tests which rely on "failOnPoisonedFieldLookup", or rewrite them so that they depend less on the internal details of how an SBE plan is structured. |
| Comments |
| Comment by Githook User [ 09/Feb/23 ] |
|
Author: {'name': 'Projjal Chanda', 'email': 'projjal.chanda@mongodb.com', 'username': 'projjal'}Message: |
| Comment by David Storch [ 21/Nov/22 ] |
|
Sending this to the triage queue, though I would put in a vote for putting it on the backlog with "neweng" status. |