Logging skips lazy aggregate field enumeration#3054
Open
anderson-joyle wants to merge 1 commit intomainfrom
Open
Logging skips lazy aggregate field enumeration#3054anderson-joyle wants to merge 1 commit intomainfrom
anderson-joyle wants to merge 1 commit intomainfrom
Conversation
CheckResult.ApplyGetLogging funnels through StructuralPrint.IsExpandedType, which called DType.AggregateHasExpandedType for every FirstName reference. For lazy aggregate types (DKind.LazyRecord / LazyTable used by CDP-style data sources), GetAllNames forces per-field resolution via LazyTypeProvider.TryGetFieldType, potentially materializing relationship metadata that a logging call should never need. Short-circuit lazy types in StructuralPrint.IsExpandedType so no TryGetFieldType calls happen on the logging path. Non-lazy aggregate behavior is unchanged: the '#$fne$#' marker continues to fire exactly as before for the dominant (non-CDP) population of expressions, and all 223 existing CheckResult + Formatter tests pass unchanged. Note for consumers that pattern-match on '#$fne$#': for FirstName references whose bound type is a lazy aggregate whose lazily-resolved fields would have contained an expand entity, the marker is no longer emitted. A follow-up can restore marker parity for lazy aggregates by adding a non-materializing predicate to LazyTypeProvider. Adds LazyLoggingSpyRecordType test helper and ApplyGetLoggingDoesNotEnumerateLazyFields regression test. Spy asserts TryGetFieldTypeCallCount and FieldNamesIterationCount stay at zero after ApplyGetLogging; without the fix the counter reaches 2 (one per field). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
✅ No public API change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: #2858
CheckResult.ApplyGetLogging funnels through StructuralPrint.IsExpandedType, which called DType.AggregateHasExpandedType for every FirstName reference. For lazy aggregate types (DKind.LazyRecord / LazyTable used by CDP-style data sources), GetAllNames forces per-field resolution via LazyTypeProvider.TryGetFieldType, potentially materializing relationship metadata that a logging call should never need.
Short-circuit lazy types in StructuralPrint.IsExpandedType so no TryGetFieldType calls happen on the logging path. Non-lazy aggregate behavior is unchanged: the '#$fne$#' marker continues to fire exactly as before for the dominant (non-CDP) population of expressions, and all 223 existing CheckResult + Formatter tests pass unchanged.
Note for consumers that pattern-match on '#$fne$#': for FirstName references whose bound type is a lazy aggregate whose lazily-resolved fields would have contained an expand entity, the marker is no longer emitted. A follow-up can restore marker parity for lazy aggregates by adding a non-materializing predicate to LazyTypeProvider.
Adds LazyLoggingSpyRecordType test helper and
ApplyGetLoggingDoesNotEnumerateLazyFields regression test. Spy asserts TryGetFieldTypeCallCount and FieldNamesIterationCount stay at zero after ApplyGetLogging; without the fix the counter reaches 2 (one per field).