From a4fb32ae4d4553965e0dc5b71dd529b275cde3bd Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Thu, 29 Aug 2024 16:57:06 -0500 Subject: [PATCH] Checking primary keys when patching --- .../Localization/Strings.cs | 1 + .../Texl/Builtins/Patch.cs | 13 +++++-- .../Utils/MutationUtils.cs | 38 +++++++++++++++---- src/strings/PowerFxResources.en-US.resx | 4 ++ .../ExpressionTestCases/Patch_V1Compat.txt | 8 ++-- .../MutationFunctionsTests.cs | 17 ++++++++- 6 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs index 68008bb151..b26cba2231 100644 --- 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 index 810f781300..2664d41f46 100644 --- 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 index e0d2df8089..3fded8f891 100644 --- 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 index 3f818dcd1a..016b4b2158 100644 --- a/src/strings/PowerFxResources.en-US.resx +++ b/src/strings/PowerFxResources.en-US.resx @@ -4756,4 +4756,8 @@ Unsupported untyped/JSON conversion type '{0}' in argument. Error Message shown to user when a unsupported type is passed in type argument of AsType, IsType and ParseJSON functions. + + Patch single record or table of single records is invalid for tables/records with no primary key. + 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. + \ No newline at end of file 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 index d52d56892b..42f73ab5ea 100644 --- 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 index 93d62e2b4d..008df3181c 100644 --- 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(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); + } + /// /// Meant to test PatchSingleRecordCoreAsync override. Only tables with primary key column are supported. ///