Prototype of Firestore query explain functionality #14079
Conversation
DO NOT MERGE This is just to demonstrate initial prototyping. Things that need fixing: - Rename QueryProfileInfo, probably to ExplainResults to match googleapis/java-firestore#1609 - Create new wrapper types for the various outputs (plan summary and explain metrics). - Possibly create a new wrapper type for ExplainOptions, and expose a single new method accepting that instead of two new methods that vary by name.
jskeet
added
the
do not merge
There was a problem hiding this comment.
Yep, this looks good.
+1 On Exposing or wrapping ExplainOptions and having a single Explain method that takes those options.
I'd even consider a SnapshotOptions that contains ExplainOptions and have Snapshot overloads that take the SnapshotOptions and return an "AugmentedSnapshot". That'd make us fully future proof. But, it could get complex, specially the return type AugmentedSnapshot.
| @@ -3,59 +3,65 @@ Microsoft Visual Studio Solution File, Format Version 12.00 | |||
| # Visual Studio Version 17 | |||
There was a problem hiding this comment.
Not sure why this changed? Maybe because of the 1PP work?
There was a problem hiding this comment.
Yes - VS has basically been inconsistent for a while, but now we're generating the sln files ourselves, so we'll definitely end up with FAE04EC0-301F-11D3-BF4B-00C04F79EFBC.
| }, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| GaxPreconditions.CheckState(readTime != null, "The stream returned from RunQuery did not provide a read timestamp."); | ||
| GaxPreconditions.CheckState(readTime is not null || explainOptions?.Analyze == false, "The stream returned from RunQuery did not provide a read timestamp."); |
There was a problem hiding this comment.
So Analyze == false is like a dry run?
| var plan = queryProfileInfo.Plan; | ||
| var stats = queryProfileInfo.Stats; | ||
| Assert.NotNull(plan); | ||
| Assert.NotNull(stats); |
There was a problem hiding this comment.
| Assert.NotNull(stats); | |
| Assert.NotNull(stats); | |
| Assert.NotNull(snapshot.ReadTime) |
DO NOT MERGE
This is just to demonstrate initial prototyping. Things that need fixing: