Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checking primary keys when patching #2621

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
13 changes: 10 additions & 3 deletions src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,…)
Expand Down
38 changes: 31 additions & 7 deletions src/libraries/Microsoft.PowerFx.Core/Utils/MutationUtils.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down Expand Up @@ -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;
}
}
}
4 changes: 4 additions & 0 deletions src/strings/PowerFxResources.en-US.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
adithyaselv marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>
Expand Down