groundwork for aggregate

DapperLib · Nov 17, 2023 · 039fb24 · 039fb24
1 parent 217ddc8
commit 039fb24
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 1 deletion.
diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs
@@ -94,6 +94,7 @@ public static readonly DiagnosticDescriptor
         VariableParameterConflict = SqlError("DAP240", "Parameter/variable conflict", "The declaration of variable '{0}' conflicts with a parameter"),
         InterpolatedStringSqlExpression = SqlWarning("DAP241", "Interpolated string usage", "Data values should not be interpolated into SQL string - use parameters instead"),
         ConcatenatedStringSqlExpression = SqlWarning("DAP242", "Concatenated string usage", "Data values should not be concatenated into SQL string - use parameters instead"),
-        InvalidDatepartToken = SqlWarning("DAP243", "Valid datepart token expected", "Date functions require a recognized datepart argument");
+        InvalidDatepartToken = SqlWarning("DAP243", "Valid datepart token expected", "Date functions require a recognized datepart argument"),
+            OnSelectAggregateAndNonAggregate = SqlWarning("DAP244", "SELECT aggregate mismatch", "SELECT has mixture of aggregate and non-aggregate expressions");
     }
 }
diff --git a/src/Dapper.AOT.Analyzers/Internal/DiagnosticTSqlProcessor.cs b/src/Dapper.AOT.Analyzers/Internal/DiagnosticTSqlProcessor.cs
@@ -139,6 +139,8 @@ protected override void OnSelectFirstTopError(Location location)
         => OnDiagnostic(DapperAnalyzer.Diagnostics.SelectFirstTopError, location);
     protected override void OnSelectSingleRowWithoutWhere(Location location)
         => OnDiagnostic(DapperAnalyzer.Diagnostics.SelectSingleRowWithoutWhere, location);
+    protected override void OnSelectAggregateAndNonAggregate(Location location)
+        => OnDiagnostic(DapperAnalyzer.Diagnostics.OnSelectAggregateAndNonAggregate, location);
     protected override void OnSelectSingleTopError(Location location)
         => OnDiagnostic(DapperAnalyzer.Diagnostics.SelectSingleTopError, location);
 

diff --git a/src/Dapper.AOT.Analyzers/SqlAnalysis/TSqlProcessor.cs b/src/Dapper.AOT.Analyzers/SqlAnalysis/TSqlProcessor.cs
@@ -238,6 +238,8 @@ protected virtual void OnSelectFirstTopError(Location location)
         => OnError($"SELECT for First* should use TOP 1", location);
     protected virtual void OnSelectSingleRowWithoutWhere(Location location)
         => OnError($"SELECT for single row without WHERE or (TOP and ORDER BY)", location);
+    protected virtual void OnSelectAggregateAndNonAggregate(Location location)
+        => OnError($"SELECT has mixture of aggregate and non-aggregate expressions", location);
     protected virtual void OnNonPositiveTop(Location location)
         => OnError($"TOP literals should be positive", location);
     protected virtual void OnNonPositiveFetch(Location location)
@@ -780,13 +782,15 @@ public override void Visit(SelectStatement node)
                 var checkNames = ValidateSelectNames;
                 HashSet<string> names = checkNames ? new HashSet<string>(StringComparer.InvariantCultureIgnoreCase) : null!;
 
+                var aggregate = AggregateFlags.None;
                 int index = 0;
                 foreach (var el in spec.SelectElements)
                 {
                     switch (el)
                     {
                         case SelectStarExpression:
                             parser.OnSelectStar(el);
+                            aggregate |= AggregateFlags.HaveNonAggregate;
                             reads++;
                             break;
                         case SelectScalarExpression scalar:
@@ -810,6 +814,7 @@ public override void Visit(SelectStatement node)
                                     parser.OnSelectDuplicateColumnName(scalar, name!);
                                 }
                             }
+                            aggregate |= IsAggregate(scalar.Expression);
                             reads++;
                             break;
                         case SelectSetVariable:
@@ -819,6 +824,11 @@ public override void Visit(SelectStatement node)
                     index++;
                 }
 
+                if (aggregate == (AggregateFlags.HaveAggregate | AggregateFlags.HaveNonAggregate))
+                {
+                    parser.OnSelectAggregateAndNonAggregate(spec);
+                }
+
                 if (reads != 0)
                 {
                     if (sets != 0)
@@ -845,6 +855,7 @@ public override void Visit(SelectStatement node)
                             // or a TOP + ORDER BY
                             if (!IsUnfiltered(spec.FromClause, spec.WhereClause)) { } // fine
                             else if (haveTopOrFetch && spec.OrderByClause is not null) { } // fine
+                            else if ((aggregate & (AggregateFlags.HaveAggregate | AggregateFlags.Uncertain)) != 0) { } // fine
                             else
                             {
                                 parser.OnSelectSingleRowWithoutWhere(node);
@@ -857,6 +868,22 @@ public override void Visit(SelectStatement node)
             base.Visit(node);
         }
 
+        enum AggregateFlags
+        {
+            None = 0,
+            HaveAggregate = 1 << 0,
+            HaveNonAggregate = 1 << 1,
+            Uncertain = 1 <<  2,
+        }
+        private AggregateFlags IsAggregate(ScalarExpression expression)
+        {
+            // any use of an aggregate function contributes HaveAggregate
+            // - there could be unary/binary operations on that aggregate function
+            // - column references inside the aggregate expression or a sub-query are fine,
+            //   otherwise they contribute HaveNonAggregate
+            return AggregateFlags.Uncertain;
+        }
+
         private bool EnforceTop(ScalarExpression expr)
         {
             if (IsInt32(expr, out var i) == TryEvaluateResult.SuccessConstant && i.HasValue)