tmenier / AsyncPoco

A long-"awaited" fully asynchronous PetaPoco fork
Other
127 stars 33 forks source link

Column-specific updates on POCOs with single primary keys fail #20

Closed jameswilddev closed 6 years ago

jameswilddev commented 9 years ago

Our application fails in the development environment, acting very strangely and not updating tables/etc. On deeper inspection, it seems there's been a regression in how AsyncPoco generates update SQL.

using AsyncPoco;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace asyncpocorepro
{
    class Program
    {
        [TableName("Test")]
        [PrimaryKey("ID")]
        [ExplicitColumns]
        public sealed class TestRecord
        {
            [Column]
            public int ID { get; set; }

            [Column]
            public string ToUpdate { get; set; }

            [Column]
            public string ToNotUpdate { get; set; }
        }

        static void Main(string[] args)
        {
            new Database("ConnectionString").UpdateAsync(new TestRecord { ID = 30, ToUpdate = "Update To" }, new[] { "ToUpdate" }).Wait();

        }
    }
}

Stepping through this, the SQL it generates is:

UPDATE [Test] SET [ToUpdate] = @0 WHERE [ID] IS NULL  --Wrong!

The reason for this is that the code path for when columns have not been specified in

public async Task<int> UpdateAsync(string tableName, string primaryKeyName, object poco, object primaryKeyValue, IEnumerable<string> columns)

scans through the list of columns, and if any are marked as a primary key, it copies their values to the primaryKeyValuePairs dictionary. However, there is no equivalent in the code path for when a list of columns is specified.

PetaPoco's source has an IF block after building the SET part of the UPDATE statement, which finds the primary key and extracts the value:

// Grab primary key value
if (primaryKeyValue == null)
{
    var pc = pd.Columns[primaryKeyName];
    primaryKeyValue = pc.GetValue(poco);
}

This is absent in AsyncPoco, thus the WHERE clause always contains nothing but NULLs.

I've replaced it with the following to grab multiple primary keys out. I'm not using multiple primary keys though so I don't know if it works with them. However, it seems to make updating work again.

foreach (var i in pd.Columns)
{
    // Don't update the primary key, but grab the value if we don't have it
    if (primaryKeyValue == null && primaryKeyValuePairs.ContainsKey(i.Key))
    {
        primaryKeyValuePairs[i.Key] = i.Value.GetValue(poco);
    }
}

Is there test coverage for this kind of scenario? Seems like the kind of thing that'd be caught.

bamboo82 commented 9 years ago

I encountered same issue. Fixed it in source codes. some bug were introduced when multiple PK support were added. Also the multiple PK update routine could be optimized for more speed.

tmenier commented 8 years ago

Sorry I never responded to this. I'm working through some minor updates to this project and will fix this if it's a bug. @bamboo82 Do you remember what the fix was? I would gladly accept a PR for this.