Skip to content
Open

Fix #530

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
54 changes: 37 additions & 17 deletions src/EFCore.Ydb/src/Update/Internal/YdbUpdateSqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Update;

namespace EntityFrameworkCore.Ydb.Update.Internal;
Expand All @@ -18,9 +19,8 @@
var name = command.TableName;
var schema = command.Schema;
var operations = command.ColumnModifications;

var writeOperations = operations.Where(o => o.IsWrite).ToList();
var readOperations = operations.Where(o => o.IsRead).ToList();
var writeOperations = operations.Where(o => o.IsWrite && !IsStoreGeneratedAndIgnoredBeforeSave(o)).ToList();
var readOperations = operations.Where(o => o.IsRead||IsStoreGeneratedAndIgnoredBeforeSave(o)).ToList();

AppendInsertCommand(
commandStringBuilder,
Expand All @@ -42,10 +42,9 @@
var name = command.TableName;
var schema = command.Schema;
var operations = command.ColumnModifications;

var writeOperations = operations.Where(o => o.IsWrite).ToList();
var writeOperations = operations.Where(o => o.IsWrite && !IsStoreGeneratedAndIgnoredBeforeSave(o)).ToList();
var conditionOperations = operations.Where(o => o.IsCondition).ToList();
var readOperations = operations.Where(o => o.IsRead).ToList();
var readOperations = operations.Where(o => o.IsRead||IsStoreGeneratedAndIgnoredBeforeSave(o)).ToList();

requiresTransaction = false;

Expand Down Expand Up @@ -105,7 +104,16 @@
bool appendReturningOneClause = false
)
{
AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations);
var effectiveWrites = writeOperations;
if (effectiveWrites.Count == 0)
{
var noOpColumn = GetNoOpSetColumn(conditionOperations, readOperations);
AppendUpdateCommandHeader(commandStringBuilder, name, schema, noOpColumn is null ? effectiveWrites : new[] { noOpColumn });
}
else
{
AppendUpdateCommandHeader(commandStringBuilder, name, schema, effectiveWrites);
}
AppendWhereClause(commandStringBuilder, conditionOperations);
AppendReturningClause(commandStringBuilder, readOperations);
commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator);
Expand Down Expand Up @@ -137,17 +145,12 @@
string? additionalValues = null
)
{
if (operations.Count <= 0) return;

commandStringBuilder
.AppendLine()
.Append("RETURNING ");
if (operations.Count <= 0 && string.IsNullOrEmpty(additionalValues)) return;

commandStringBuilder.AppendJoin(
',',
operations
.Select(operation => SqlGenerationHelper.DelimitIdentifier(operation.ColumnName))
);
commandStringBuilder.AppendLine().Append("RETURNING ");
var columns = operations.Select(o => SqlGenerationHelper.DelimitIdentifier(o.ColumnName)).ToList();
if (!string.IsNullOrEmpty(additionalValues)) columns.Add(additionalValues!);

Check warning on line 152 in src/EFCore.Ydb/src/Update/Internal/YdbUpdateSqlGenerator.cs

View workflow job for this annotation

GitHub Actions / Inspection (./src/YdbSdk.sln)

"[RedundantSuppressNullableWarningExpression] The nullable warning suppression expression is redundant" on /home/runner/work/ydb-dotnet-sdk/ydb-dotnet-sdk/src/EFCore.Ydb/src/Update/Internal/YdbUpdateSqlGenerator.cs(152,82)
commandStringBuilder.AppendJoin(',', columns);
}

public override string GenerateNextSequenceValueOperation(string name, string? schema)
Expand All @@ -163,4 +166,21 @@
public override void AppendObtainNextSequenceValueOperation(
StringBuilder commandStringBuilder, string name, string? schema
) => throw new NotSupportedException("Iterating over serial is not supported in YDB");

private static bool IsStoreGeneratedAndIgnoredBeforeSave(IColumnModification op)
{
var p = op.Property;
if (p == null) return false;
if (p.ValueGenerated != ValueGenerated.OnAdd && p.ValueGenerated != ValueGenerated.OnAddOrUpdate) return false;
return p.GetBeforeSaveBehavior() == PropertySaveBehavior.Ignore;
}

private static IColumnModification? GetNoOpSetColumn(
IReadOnlyList<IColumnModification> conditionOperations,
IReadOnlyList<IColumnModification> readOperations
)
{
var candidate = conditionOperations.FirstOrDefault() ?? readOperations.FirstOrDefault();
return candidate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@

namespace EntityFrameworkCore.Ydb.FunctionalTests.Update;

// Tests:
// Ignore_before_save_property_is_still_generated_graph,
// Ignore_before_save_property_is_still_generated,
// SaveChanges_processes_all_tracked_entities.
// They're failing, but I cannot ignore them because they're not virtual
#pragma warning disable xUnit1000
internal class UpdatesYdbTest
#pragma warning restore xUnit1000
Expand Down Expand Up @@ -56,7 +51,7 @@ public override Task Swap_computed_unique_index_values()
public override Task Update_non_indexed_values()
=> TestIgnoringBase(base.Update_non_indexed_values);

[ConditionalTheory(Skip = "TODO: need fix")]
[Theory]
[InlineData(false)]
[InlineData(true)]
public override Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
Expand Down
Loading