Checking primary keys when patching

microsoft · Sep 9, 2024 · a4fb32a · a4fb32a
1 parent 3c7c076
commit a4fb32a
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 15 deletions.
diff --git a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs
@@ -843,5 +843,6 @@ internal static class TexlStrings
         public static ErrorResourceKey ErrInvalidDataSourceForFunction = new ErrorResourceKey("ErrInvalidDataSourceForFunction");
         public static ErrorResourceKey ErrInvalidArgumentExpectedType = new ErrorResourceKey("ErrInvalidArgumentExpectedType");
         public static ErrorResourceKey ErrUnsupportedTypeInTypeArgument = new ErrorResourceKey("ErrUnsupportedTypeInTypeArgument");
+        public static ErrorResourceKey ErrPatchSingleRecordInvalidTableRecord = new ErrorResourceKey("ErrPatchSingleRecordInvalidTableRecord");
     }
 }
diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs
@@ -10,13 +10,10 @@
 using Microsoft.PowerFx.Core.Errors;
 using Microsoft.PowerFx.Core.Functions;
 using Microsoft.PowerFx.Core.Functions.DLP;
-using Microsoft.PowerFx.Core.IR.Nodes;
-using Microsoft.PowerFx.Core.IR.Symbols;
 using Microsoft.PowerFx.Core.Localization;
 using Microsoft.PowerFx.Core.Types;
 using Microsoft.PowerFx.Core.Utils;
 using Microsoft.PowerFx.Syntax;
-using static Microsoft.PowerFx.Core.IR.IRTranslator;
 using CallNode = Microsoft.PowerFx.Syntax.CallNode;
 
 namespace Microsoft.PowerFx.Core.Texl.Builtins
@@ -474,6 +471,11 @@ public PatchSingleRecordFunction()
         {
             yield return new[] { TexlStrings.PatchArg_Source, TexlStrings.PatchArg_Record };
         }
+
+        public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors)
+        {
+            MutationUtils.CheckPrimaryKeys(args, argTypes, errors);
+        }
     }
 
     // Patch(DS, table_of_rows, table_of_updates)
@@ -516,6 +518,11 @@ public PatchAggregateSingleTableFunction()
         {
             yield return new[] { TexlStrings.PatchArg_Source, TexlStrings.PatchArg_Rows };
         }
+
+        public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors)
+        {
+            MutationUtils.CheckPrimaryKeys(args, argTypes, errors);
+        }
     }
 
     // Patch(Record, Updates1, Updates2,…)

diff --git a/src/libraries/Microsoft.PowerFx.Core/Utils/MutationUtils.cs b/src/libraries/Microsoft.PowerFx.Core/Utils/MutationUtils.cs
@@ -1,22 +1,15 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-using System.Collections.Generic;
 using System.Linq;
 using Microsoft.PowerFx.Core.App.ErrorContainers;
 using Microsoft.PowerFx.Core.Binding;
 using Microsoft.PowerFx.Core.Entities;
 using Microsoft.PowerFx.Core.Errors;
 using Microsoft.PowerFx.Core.Functions;
-using Microsoft.PowerFx.Core.IR;
-using Microsoft.PowerFx.Core.IR.Nodes;
-using Microsoft.PowerFx.Core.IR.Symbols;
 using Microsoft.PowerFx.Core.Localization;
 using Microsoft.PowerFx.Core.Types;
 using Microsoft.PowerFx.Syntax;
-using Microsoft.PowerFx.Types;
-using CallNode = Microsoft.PowerFx.Syntax.CallNode;
-using RecordNode = Microsoft.PowerFx.Core.IR.Nodes.RecordNode;
 
 namespace Microsoft.PowerFx.Core.Utils
 {
@@ -70,5 +63,36 @@ public static void CheckSemantics(TexlBinding binding, TexlFunction function, Te
                 return;
             }
         }
+
+        public static bool CheckPrimaryKeys(TexlNode[] args, DType[] argTypes, IErrorContainer errors)
+        {
+            var arg0 = argTypes[0];
+            var arg1 = argTypes[1];
+
+            if (arg0.AssociatedDataSources.Any() &&
+                arg0.AssociatedDataSources.Single() is IExternalTabularDataSource externalTabularDataSource)
+            {
+                if (!externalTabularDataSource.GetKeyColumns().Any())
+                {
+                    errors.EnsureError(args[0], TexlStrings.ErrPatchSingleRecordInvalidTableRecord);
+                    return false;
+                }
+
+                var primaryKeys = externalTabularDataSource.GetKeyColumns();
+                var arg1Names = arg1.GetNames(DPath.Root).Select(tn => tn.Name.Value);
+                var missingPrimaryKey = primaryKeys.Where(name => !arg1Names.Contains(name)).Any();
+
+                if (missingPrimaryKey)
+                {
+                    errors.EnsureError(args[0], TexlStrings.ErrPatchSingleRecordInvalidTableRecord);
+                    return false;
+                }
+
+                return true;
+            }
+
+            errors.EnsureError(args[0], TexlStrings.ErrPatchSingleRecordInvalidTableRecord);
+            return false;
+        }
     }
 }
diff --git a/src/strings/PowerFxResources.en-US.resx b/src/strings/PowerFxResources.en-US.resx
@@ -4756,4 +4756,8 @@
     <value>Unsupported untyped/JSON conversion type '{0}' in argument.</value>
     <comment>Error Message shown to user when a unsupported type is passed in type argument of AsType, IsType and ParseJSON functions.</comment>
   </data>
+  <data name="ErrPatchSingleRecordInvalidTableRecord" xml:space="preserve">
+    <value>Patch single record or table of single records is invalid for tables/records with no primary key.</value>
+    <comment>Error message shown to user when there is an attempt of calling Patch(table, single_record) function where table/record is not a dataverse one.</comment>
+  </data>
 </root>
diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Patch_V1Compat.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Patch_V1Compat.txt
@@ -272,17 +272,17 @@ Table({Field1:123,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},
 // **********************************************************
 
 >> Patch(t1, Table(r1))
-Error({Kind:ErrorKind.NotSupported})
+Errors: Error 6-8: Patch single record or table of single records is invalid for tables/records with no primary key.
 
 >> Patch(t1, If(false,t1))
-Blank()
+Errors: Error 6-8: Patch single record or table of single records is invalid for tables/records with no primary key.
 
 
 // **********************************************************
 // Patch(DS, record_with_keys_and_updates)
 // **********************************************************
 >> Patch(t1, r1)
-Error({Kind:ErrorKind.NotSupported})
+Errors: Error 6-8: Patch single record or table of single records is invalid for tables/records with no primary key.
 
 >> Patch(t1, If(false,r1))
-Blank()
+Errors: Error 6-8: Patch single record or table of single records is invalid for tables/records with no primary key.
diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationFunctionsTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationFunctionsTests.cs
@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Data;
-using System.Drawing;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -517,6 +516,22 @@ public void AppendErrorTests()
             Assert.IsType<ErrorValue>(result);
         }
 
+        [Theory]
+        [InlineData(
+            "Patch(t1, {name:\"Mary Doe\"});Concat(t1, $\"{name} from {address1_city}\", \",\")",
+            "ErrPatchSingleRecordInvalidTableRecord")]
+        [InlineData(
+            "Patch(t1, Table({name:\"Mary Doe\"}));Concat(t1, $\"{name} from {address1_city}\", \",\")",
+            "ErrPatchSingleRecordInvalidTableRecord")]
+        public void PatchEntityMissingPrimaryKeysTests(string expression, string expectedError)
+        {
+            var engine = PatchEngine;
+            var check = engine.Check(expression, options: new ParserOptions() { AllowsSideEffects = true });
+
+            Assert.False(check.IsSuccess);
+            Assert.Equal(expectedError, check.Errors.First().MessageKey);
+        }
+
         /// <summary>
         /// Meant to test PatchSingleRecordCoreAsync override. Only tables with primary key column are supported.
         /// </summary>