From ceca74a6560bfed450270e9df4f26ec9c7530bc6 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Tue, 21 Apr 2026 14:20:56 -0500 Subject: [PATCH] Fix #2858: ApplyGetLogging skips lazy aggregate field enumeration 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) --- .../Logging/StructuralPrint.cs | 8 ++- .../CheckResultTests.cs | 27 ++++++++++ .../LazyLoggingSpyRecordType.cs | 52 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 src/tests/Microsoft.PowerFx.Core.Tests.Shared/LazyLoggingSpyRecordType.cs diff --git a/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs b/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs index a1b9d2384a..9c6c52eb31 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Logging/StructuralPrint.cs @@ -483,7 +483,13 @@ private bool IsExpandedType(TexlBinding binding, TexlNode node) { var type = _binding?.GetTypeAllowInvalid(node); - return type != null && type.AggregateHasExpandedType(); + if (type == null) + { + return false; + } + + // Skip lazy aggregates: AggregateHasExpandedType forces per-field materialization. + return !type.IsLazyType && type.AggregateHasExpandedType(); } } } diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/CheckResultTests.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/CheckResultTests.cs index a3ed3bebcc..687a5923c4 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/CheckResultTests.cs +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/CheckResultTests.cs @@ -559,6 +559,33 @@ public void ApplyLoggingWithBinding(string expr) var anon = check.ApplyGetLogging(); } + [Fact] + public void ApplyGetLoggingDoesNotEnumerateLazyFields() + { + var spy = new LazyLoggingSpyRecordType(); + var symbolTable = new SymbolTable(); + symbolTable.AddVariable("MyLazy", spy); + + CheckResult check = new CheckResult(new Engine()) + .SetText("With({x:MyLazy}, 1)") + .SetBindingInfo(symbolTable); + + check.ApplyBinding(); + Assert.True(check.IsSuccess); + + // Reset counters so we observe only the logging path. + spy.TryGetFieldTypeCallCount = 0; + spy.FieldNamesIterationCount = 0; + + var log = check.ApplyGetLogging(); + + Assert.NotNull(log); + Assert.DoesNotContain("MyLazy", log); + Assert.DoesNotContain("A", log); + Assert.Equal(0, spy.TryGetFieldTypeCallCount); + Assert.Equal(0, spy.FieldNamesIterationCount); + } + [Fact] public void TestSummary() { diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/LazyLoggingSpyRecordType.cs b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/LazyLoggingSpyRecordType.cs new file mode 100644 index 0000000000..1591aaab0a --- /dev/null +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/LazyLoggingSpyRecordType.cs @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Collections.Generic; +using Microsoft.PowerFx.Types; + +namespace Microsoft.PowerFx.Core.Tests +{ + // Test helper. Lets tests assert that the logging path does not enumerate + // lazy record fields or force per-field type resolution. + public sealed class LazyLoggingSpyRecordType : RecordType + { + public int TryGetFieldTypeCallCount; + public int FieldNamesIterationCount; + + public override IEnumerable FieldNames => EnumerateFieldNames(); + + public override bool TryGetFieldType(string name, out FormulaType type) + { + TryGetFieldTypeCallCount++; + switch (name) + { + case "A": + type = FormulaType.String; + return true; + case "B": + type = FormulaType.Number; + return true; + default: + type = FormulaType.Blank; + return false; + } + } + + public override bool Equals(object other) + { + return other is LazyLoggingSpyRecordType; + } + + public override int GetHashCode() + { + return 0x511F; + } + + private IEnumerable EnumerateFieldNames() + { + FieldNamesIterationCount++; + yield return "A"; + yield return "B"; + } + } +}