Fixes incorrect query on string overflow

src/Microsoft.Health.Fhir.Core/Features/Definition/ISearchParameterDefinitionManager.cs

@@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using Microsoft.Health.Fhir.Core.Models;
+using Microsoft.Health.Fhir.ValueSets;
 
 namespace Microsoft.Health.Fhir.Core.Features.Definition
 {
@@ -49,5 +50,14 @@ public interface ISearchParameterDefinitionManager
         /// <param name="definitionUri">The search parameter definition URL.</param>
         /// <returns>The search parameter with the given <paramref name="definitionUri"/>.</returns>
         SearchParameterInfo GetSearchParameter(Uri definitionUri);
+
+        /// <summary>
+        /// Gets the type of a search parameter expression. In the case of a composite search parameter, the component parameter
+        /// can be specified, to retrieve the type of that component.
+        /// </summary>
+        /// <param name="searchParameter">The search parameter</param>
+        /// <param name="componentIndex">The optional component index if the search parameter is a composite</param>
+        /// <returns>The search parameter type.</returns>
+        SearchParamType GetSearchParameterType(SearchParameterInfo searchParameter, int? componentIndex);
     }
 }

src/Microsoft.Health.Fhir.Core/Models/SearchParameterInfo.cs

@@ -14,15 +14,15 @@ public class SearchParameterInfo
     {
         public SearchParameterInfo(
             string name,
+            string searchParamType,
             Uri url = null,
-            string searchParamType = null,
             IReadOnlyList<SearchParameterComponentInfo> components = null,
             string expression = null,
             IReadOnlyCollection<string> targetResourceTypes = null)
             : this(
                 name,
+                Enum.Parse<SearchParamType>(searchParamType),
                 url,
-                Enum.TryParse<SearchParamType>(searchParamType, out var type) ? (SearchParamType?)type : null,
                 components,
                 expression,
                 targetResourceTypes)
@@ -31,8 +31,8 @@ public SearchParameterInfo(
 
         public SearchParameterInfo(
             string name,
+            SearchParamType searchParamType,
             Uri url = null,
-            SearchParamType? searchParamType = null,
             IReadOnlyList<SearchParameterComponentInfo> components = null,
             string expression = null,
             IReadOnlyCollection<string> targetResourceTypes = null)
@@ -62,7 +62,7 @@ public SearchParameterInfo(string name)
 
         public Uri Url { get; }
 
-        public SearchParamType? Type { get; }
+        public SearchParamType Type { get; }
 
         public IReadOnlyList<SearchParameterComponentInfo> Component { get; }
     }

src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Search/SearchOptionsFactoryTests.cs

@@ -33,7 +33,7 @@ public partial class SearchOptionsFactoryTests
         public SearchOptionsFactoryTests()
         {
             var searchParameterDefinitionManager = Substitute.For<ISearchParameterDefinitionManager>();
-            searchParameterDefinitionManager.GetSearchParameter(ResourceType.Resource.ToString(), SearchParameterNames.ResourceType).Returns(new SearchParameter { Name = SearchParameterNames.ResourceType }.ToInfo());
+            searchParameterDefinitionManager.GetSearchParameter(ResourceType.Resource.ToString(), SearchParameterNames.ResourceType).Returns(new SearchParameter { Name = SearchParameterNames.ResourceType, Type = SearchParamType.String }.ToInfo());
 
             _factory = new SearchOptionsFactory(
                 _expressionParser,

src/Microsoft.Health.Fhir.Shared.Core/Extensions/ModelExtensions.cs

@@ -100,8 +100,8 @@ public static SearchParameterInfo ToInfo(this SearchParameter searchParam)
 
             return new SearchParameterInfo(
                 searchParam.Name,
-                string.IsNullOrEmpty(searchParam.Url) ? null : new Uri(searchParam.Url),
                 searchParam.Type?.ToString(),
+                string.IsNullOrEmpty(searchParam.Url) ? null : new Uri(searchParam.Url),
                 searchParam.Component?.Select(x => new SearchParameterComponentInfo(x.GetComponentDefinitionUri(), x.Expression)).ToArray(),
                 searchParam.Expression,
                 searchParam.Target?.Select(x => x?.ToString()).ToArray());

src/Microsoft.Health.Fhir.Shared.Core/Features/Definition/SearchParameterDefinitionBuilder.cs

@@ -166,7 +166,7 @@ private static bool ShouldExcludeEntry(string resourceType, string searchParamet
             var validatedSearchParameters = new List<(string ResourceType, SearchParameterInfo SearchParameter)>
             {
                 // _type is currently missing from the search params definition bundle, so we inject it in here.
-                (ResourceType.Resource.ToString(), new SearchParameterInfo(SearchParameterNames.ResourceType, SearchParameterNames.ResourceTypeUri, SearchParamType.Token.ToString(), null, "Resource.type().name", null)),
+                (ResourceType.Resource.ToString(), new SearchParameterInfo(SearchParameterNames.ResourceType, SearchParamType.Token.ToString(), SearchParameterNames.ResourceTypeUri, null, "Resource.type().name", null)),
             };
 
             // Do the second pass to make sure the definition is valid.

src/Microsoft.Health.Fhir.Shared.Core/Features/Definition/SearchParameterDefinitionManager.cs

@@ -91,6 +91,19 @@ public SearchParameterInfo GetSearchParameter(Uri definitionUri)
             throw new SearchParameterNotSupportedException(definitionUri);
         }
 
+        public ValueSets.SearchParamType GetSearchParameterType(SearchParameterInfo searchParameter, int? componentIndex)
+        {
+            if (componentIndex == null)
+            {
+                return searchParameter.Type;
+            }
+
+            SearchParameterComponentInfo component = searchParameter.Component[componentIndex.Value];
+            SearchParameterInfo componentSearchParameter = GetSearchParameter(component.DefinitionUrl);
+
+            return componentSearchParameter.Type;
+        }
+
         void IProvideCapability.Build(IListedCapabilityStatement statement)
         {
             EnsureArg.IsNotNull(statement, nameof(statement));
@@ -101,7 +114,7 @@ void IProvideCapability.Build(IListedCapabilityStatement statement)
                         searchParameter => new CapabilityStatement.SearchParamComponent
                         {
                             Name = searchParameter.Key,
-                            Type = Enum.Parse<SearchParamType>(searchParameter.Value.Type?.ToString()),
+                            Type = Enum.Parse<SearchParamType>(searchParameter.Value.Type.ToString()),
                         });
 
                 var capabilityStatement = (ListedCapabilityStatement)statement;

src/Microsoft.Health.Fhir.Shared.Core/Features/Search/Expressions/Parsers/SearchParameterExpressionParser.cs

@@ -176,7 +176,7 @@ private Expression Build(
             }
 
             // Parse the value.
-            Func<string, ISearchValue> parser = _parserDictionary[Enum.Parse<SearchParamType>(searchParameter.Type?.ToString())];
+            Func<string, ISearchValue> parser = _parserDictionary[Enum.Parse<SearchParamType>(searchParameter.Type.ToString())];
 
             // Build the expression.
             var helper = new SearchValueExpressionBuilderHelper();

src/Microsoft.Health.Fhir.Shared.Core/Features/Search/SearchOptionsFactory.cs

@@ -10,7 +10,6 @@
 using Hl7.Fhir.Model;
 using Hl7.Fhir.Rest;
 using Microsoft.Extensions.Logging;
-using Microsoft.Health.Fhir.Core.Exceptions;
 using Microsoft.Health.Fhir.Core.Features.Definition;
 using Microsoft.Health.Fhir.Core.Features.Search.Expressions;
 using Microsoft.Health.Fhir.Core.Features.Search.Expressions.Parsers;

src/Microsoft.Health.Fhir.SqlServer.Api/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs

@@ -95,6 +95,10 @@ public static IFhirServerBuilder AddExperimentalSqlServer(this IFhirServerBuilde
                 .Singleton()
                 .AsSelf();
 
+            services.Add<StringOverflowRewriter>()
+                .Singleton()
+                .AsSelf();
+
             return fhirServerBuilder;
         }
 

src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/StringSearchParameterQueryGenerator.cs

@@ -28,6 +28,19 @@ public override SearchParameterQueryGeneratorContext VisitString(StringExpressio
                     break;
                 case SqlFieldName.TextOverflow:
                     column = V1.StringSearchParam.TextOverflow;
+                    switch (expression.StringOperator)
+                    {
+                        case StringOperator.StartsWith:
+                        case StringOperator.NotStartsWith:
+                        case StringOperator.Equals:
+                            if (expression.Value.Length <= V1.StringSearchParam.Text.Metadata.MaxLength)
+                            {
+                                column = V1.StringSearchParam.Text;
+                            }
+
+                            break;
+                    }
+
                     context.StringBuilder.Append(" IS NOT NULL AND ");
                     break;
                 default:

src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/StringOverflowRewriter.cs

@@ -3,8 +3,12 @@
 // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
 // -------------------------------------------------------------------------------------------------
 
+using EnsureThat;
+using Microsoft.Health.Fhir.Core.Features.Definition;
 using Microsoft.Health.Fhir.Core.Features.Search.Expressions;
+using Microsoft.Health.Fhir.Core.Models;
 using Microsoft.Health.Fhir.SqlServer.Features.Schema.Model;
+using Microsoft.Health.Fhir.ValueSets;
 
 namespace Microsoft.Health.Fhir.SqlServer.Features.Search.Expressions.Visitors
 {
@@ -14,42 +18,72 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Search.Expressions.Visitors
     /// </summary>
     internal class StringOverflowRewriter : ConcatenationRewriter
     {
-        internal static readonly StringOverflowRewriter Instance = new StringOverflowRewriter();
+        private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager;
 
-        private StringOverflowRewriter()
-            : base(new Scout())
+        public StringOverflowRewriter(ISearchParameterDefinitionManager searchParameterDefinitionManager)
+            : base(new Scout(searchParameterDefinitionManager))
         {
+            EnsureArg.IsNotNull(searchParameterDefinitionManager, nameof(searchParameterDefinitionManager));
+            _searchParameterDefinitionManager = searchParameterDefinitionManager;
+        }
+
+        public override Expression VisitSearchParameter(SearchParameterExpression expression, object context)
+        {
+            if (expression.Parameter.Type == SearchParamType.String ||
+                expression.Parameter.Type == SearchParamType.Composite)
+            {
+                return base.VisitSearchParameter(expression, expression.Parameter);
+            }
+
+            return expression;
         }
 
         public override Expression VisitString(StringExpression expression, object context)
         {
+            if (_searchParameterDefinitionManager.GetSearchParameterType((SearchParameterInfo)context, expression.ComponentIndex) != SearchParamType.String)
+            {
+                return expression;
+            }
+
             return new StringExpression(expression.StringOperator, SqlFieldName.TextOverflow, expression.ComponentIndex, expression.Value, expression.IgnoreCase);
         }
 
         private class Scout : DefaultExpressionVisitor<object, bool>
         {
-            internal Scout()
+            private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager;
+
+            internal Scout(ISearchParameterDefinitionManager searchParameterDefinitionManager)
                 : base((accumulated, current) => accumulated || current)
             {
+                EnsureArg.IsNotNull(searchParameterDefinitionManager, nameof(searchParameterDefinitionManager));
+                _searchParameterDefinitionManager = searchParameterDefinitionManager;
+            }
+
+            public override bool VisitSearchParameter(SearchParameterExpression expression, object context)
+            {
+                if (expression.Parameter.Type == SearchParamType.String ||
+                    expression.Parameter.Type == SearchParamType.Composite)
+                {
+                    return expression.Expression.AcceptVisitor(this, expression.Parameter);
+                }
+
+                return false;
             }
 
             public override bool VisitString(StringExpression expression, object context)
             {
-                switch (expression.StringOperator)
+                if (_searchParameterDefinitionManager.GetSearchParameterType((SearchParameterInfo)context, expression.ComponentIndex) != SearchParamType.String)
                 {
-                    case StringOperator.Equals:
-                    case StringOperator.NotStartsWith:
-                    case StringOperator.StartsWith:
-                        if (expression.Value.Length < V1.StringSearchParam.Text.Metadata.MaxLength / 2)
-                        {
-                            return false;
-                        }
-
-                        return true;
-
-                    default:
-                        return true;
+                    return false;
                 }
+
+                if (expression.StringOperator == StringOperator.Equals && expression.Value.Length <= V1.StringSearchParam.Text.Metadata.MaxLength)
+                {
+                    // in these cases, we will know for sure that we do not need to consider the overflow column
+                    return false;
+                }
+
+                return true;
             }
         }
     }

src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParameters.cs

@@ -15,6 +15,6 @@ public static class SqlSearchParameters
 
         public static readonly Uri ResourceSurrogateIdUri = new Uri("http://fhirserverforazure.microsoft.com/fhir/SearchParameter/Resource-surrogateid");
 
-        public static readonly SearchParameterInfo ResourceSurrogateIdParameter = new SearchParameterInfo(ResourceSurrogateIdParameterName, ResourceSurrogateIdUri, SearchParamType.Number, null);
+        public static readonly SearchParameterInfo ResourceSurrogateIdParameter = new SearchParameterInfo(ResourceSurrogateIdParameterName, SearchParamType.Number, ResourceSurrogateIdUri, null);
     }
 }

src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs

@@ -34,6 +34,7 @@ internal class SqlServerSearchService : SearchService
         private readonly SqlServerFhirModel _model;
         private readonly SqlRootExpressionRewriter _sqlRootExpressionRewriter;
         private readonly ChainFlatteningRewriter _chainFlatteningRewriter;
+        private readonly StringOverflowRewriter _stringOverflowRewriter;
         private readonly SqlServerDataStoreConfiguration _configuration;
         private readonly ILogger<SqlServerSearchService> _logger;
 
@@ -44,17 +45,20 @@ public SqlServerSearchService(
             SqlServerFhirModel model,
             SqlRootExpressionRewriter sqlRootExpressionRewriter,
             ChainFlatteningRewriter chainFlatteningRewriter,
+            StringOverflowRewriter stringOverflowRewriter,
             SqlServerDataStoreConfiguration configuration,
             ILogger<SqlServerSearchService> logger)
             : base(searchOptionsFactory, fhirDataStore, modelInfoProvider)
         {
             EnsureArg.IsNotNull(sqlRootExpressionRewriter, nameof(sqlRootExpressionRewriter));
             EnsureArg.IsNotNull(chainFlatteningRewriter, nameof(chainFlatteningRewriter));
+            EnsureArg.IsNotNull(stringOverflowRewriter, nameof(stringOverflowRewriter));
             EnsureArg.IsNotNull(logger, nameof(logger));
 
             _model = model;
             _sqlRootExpressionRewriter = sqlRootExpressionRewriter;
             _chainFlatteningRewriter = chainFlatteningRewriter;
+            _stringOverflowRewriter = stringOverflowRewriter;
             _configuration = configuration;
             _logger = logger;
         }
@@ -99,7 +103,7 @@ private async Task<SearchResult> SearchImpl(SearchOptions searchOptions, bool hi
                                                .AcceptVisitor(NormalizedPredicateReorderer.Instance)
                                                .AcceptVisitor(_chainFlatteningRewriter)
                                                .AcceptVisitor(DateTimeBoundedRangeRewriter.Instance)
-                                               .AcceptVisitor(StringOverflowRewriter.Instance)
+                                               .AcceptVisitor(_stringOverflowRewriter)
                                                .AcceptVisitor(NumericRangeRewriter.Instance)
                                                .AcceptVisitor(MissingSearchParamVisitor.Instance)
                                                .AcceptVisitor(TopRewriter.Instance, searchOptions)

src/Microsoft.Health.Fhir.Tests.Common/Microsoft.Health.Fhir.Tests.Common.csproj

@@ -68,6 +68,7 @@
     <None Remove="TestFiles\R4\ObservationWithEyeColor.json" />
     <None Remove="TestFiles\R4\ObservationWithInvalidStatus.json" />
     <None Remove="TestFiles\R4\ObservationWithInvalidStatus.xml" />
+    <None Remove="TestFiles\R4\ObservationWithLongEyeColor.json" />
     <None Remove="TestFiles\R4\ObservationWithNoCode.json" />
     <None Remove="TestFiles\R4\ObservationWithNoCode.xml" />
     <None Remove="TestFiles\R4\ObservationWithTemperature.json" />
@@ -83,6 +84,7 @@
     <None Remove="TestFiles\Stu3\DocumentReference-example-002.json" />
     <None Remove="TestFiles\Stu3\DocumentReference-example-003.json" />
     <None Remove="TestFiles\Stu3\DocumentReference-example.json" />
+    <None Remove="TestFiles\Stu3\ObservationWithLongEyeColor.json" />
     <None Remove="TestFiles\ValidCompartmentDefinition.json" />
     <None Remove="TestFiles\Weight.json" />
     <None Remove="TestFiles\WeightInGrams.json" />
@@ -119,6 +121,7 @@
     <EmbeddedResource Include="TestFiles\R4\ObservationWith1MinuteApgarScore.json" />
     <EmbeddedResource Include="TestFiles\R4\ObservationWith20MinuteApgarScore.json" />
     <EmbeddedResource Include="TestFiles\R4\ObservationWithBloodPressure.json" />
+    <EmbeddedResource Include="TestFiles\R4\ObservationWithLongEyeColor.json" />
     <EmbeddedResource Include="TestFiles\R4\ObservationWithEyeColor.json" />
     <EmbeddedResource Include="TestFiles\R4\ObservationWithInvalidStatus.json" />
     <EmbeddedResource Include="TestFiles\R4\ObservationWithInvalidStatus.xml" />
@@ -148,6 +151,7 @@
     <EmbeddedResource Include="TestFiles\Stu3\ObservationWith1MinuteApgarScore.json" />
     <EmbeddedResource Include="TestFiles\Stu3\ObservationWith20MinuteApgarScore.json" />
     <EmbeddedResource Include="TestFiles\Stu3\ObservationWithBloodPressure.json" />
+    <EmbeddedResource Include="TestFiles\Stu3\ObservationWithLongEyeColor.json" />
     <EmbeddedResource Include="TestFiles\Stu3\ObservationWithEyeColor.json" />
     <EmbeddedResource Include="TestFiles\Stu3\ObservationWithTemperature.json" />
     <EmbeddedResource Include="TestFiles\Stu3\ObservationWithTPMTDiplotype.json" />

src/Microsoft.Health.Fhir.Tests.Common/TestFiles/R4/ObservationWithLongEyeColor.json

@@ -0,0 +1,24 @@
+{
+    "resourceType": "Observation",
+    "id": "eye-color",
+    "text": {
+        "status": "generated",
+        "div": "<div xmlns=\"http://www.w3.org/1999/xhtml\"><p><b>Generated Narrative with Details</b></p><p><b>id</b>: eye-color</p><p><b>status</b>: final</p><p><b>code</b>: eye color <span>(Details )</span></p><p><b>subject</b>: <a>Patient/example</a></p><p><b>effective</b>: 18/05/2016</p><p><b>value</b>: blue</p></div>"
+    },
+    "status": "final",
+    "code": {
+        "coding": [
+            {
+                "system": "http://snomed.info/sct",
+                "code": "162806009",
+                "display": "On examination - general eye examination (finding)"
+            }
+        ],
+        "text": "eye color"
+    },
+    "subject": {
+        "reference": "Patient/example"
+    },
+    "effectiveDateTime": "2016-05-18",
+    "valueString": "Lorem ipsum dolor sit amet consectetur adipiscing elit. Ut eget ultricies justo. Maecenas bibendum convallis sodales. Vestibulum quis molestie dui. Nulla porta elementum tristique. Aenean neque libero convallis sit amet dui ullamcorper congue lacinia erat. Sed finibus ex ac massa tincidunt tristique. In sed auctor massa. Proin cursus porttitor arcu. Maecenas a leo nunc. Sed pretium porta volutpat. In aliquet tempor sapien vitae laoreet nisl tempor ac. Vestibulum lacus leo luctus vitae pharetra at tempus ac diam. Integer at dui eu dolor gravida vehicula. Phasellus malesuada elit orci quis maximus purus consectetur ac. In semper consequat augue sit amet ultricies."
+}