[SERVER-55117] Consolidate LiteParsePipeline::validate() and Pipeline::validateCommon() Created: 10/Mar/21 Updated: 06/Dec/22 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Arun Banala | Assignee: | Backlog - Query Optimization |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Query Optimization
|
| Sprint: | Query Optimization 2021-05-17 |
| Participants: |
| Description |
|
It seems like there is some ambiguity about where new pipeline stage related validations should go. We might be able to consolidate LiteParsePipeline::validate() and Pipeline::validateCommon() and move all of the validations to Pipeline::validateCommon() |
| Comments |
| Comment by Charlie Swanson [ 03/Jun/21 ] |
|
Based on hana.pearlman's comment we're going to put this on the back burner for a while. Let us know if you have any objections arun.banala |
| Comment by Hana Pearlman [ 27/May/21 ] |
|
After investigating: It should be possible to do this without too much difficulty! The main work is in moving some document source information from the LiteParsedDocumentSource parser map to the DocumentSource parser map. One challenge is that LiteParsePipeline::validate() validates both the top-level pipeline and any subpipelines. But, since Pipeline::validateCommon() happens at the parse-time of the top-level pipeline and since some stages have subpipelines that cannot be parsed until later, we can't do the same validation on the subpipelines. I think it should be OK to ignore any subpipelines in Pipeline::validateCommon() and rely on that validation happening when the subpipelines themselves are parsed. This is fine in most cases, but there may be some issues around API version checks. Ideally we could roll these checks into the validateCommon() method as well, but they are skipped for time-series views. I wasn't able to immediately see where we should do API version checks to ensure we also get coverage anywhere a sub-pipeline may be parsed (without pasting an extra function call after nearly every Pipeline::parse()). |