Inconsistency between Update and UpdateOnly

Introduction
We run into an issue where we need to have parameter values set to lowercase for search queries. This is especially useful when querying string values in a Postgres DB. We use lower(…) indexes on search columns.

Problem description
When using Update for any POCO/Entity, all works fine and all values in the DB are set to the exact values in the update fields. When using UpdateOnly for any POCO/Entity, we run into an issue, since all string values are set to lowercase. So, UPDATE is OK, UPDATEONLY fails.

Reason this happens
We created our ‘own’ PostgreSqlDialectProvider which overrides GetParamValue for string values.

public override object GetParamValue(object value, Type fieldType)
{
  var ret = base.GetParamValue(value, fieldType);
  if (value != null && fieldType == typeof(string))
  {
    return ret.ToString().ToLower();
  }
  return ret;
}

This allows for ‘automagically’ setting al string search parameters to lowercase. Perfect!

Following the code in OrmLite, I found that the Update on a complete POCO uses GetFieldValue to hydrate the parameters’ values for the update, whereas UpdateOnly uses GetParamValue to hydrate the parameters’ values. The latter has a side effect of the DialectProvider setting all values to lowercase.

We would very much like to see the UpdateOnly to also use the GetFieldValue instead of the GetParameterValue. My understanding of GetParameterValue is that is would be used only for select queries.

GetParamValue() definitely wasn’t designed to be limited to select parameters, but can you point to the code in question by clicking on line number in GitHub and pasting the link here.

Yes. Working on ‘a possible fix’ now. Will report when done.

OK, managed to find a ‘fix’ for the UpdateOnly variant, but I am not completely happy with it. The change is in the file OrmLiteDialectProvidedBase.cs:

    public virtual void PrepareUpdateRowStatement(IDbCommand dbCmd, object objWithProperties, ICollection<string> updateFields = null)
    {
        var sql = StringBuilderCache.Allocate();
        var sqlFilter = StringBuilderCacheAlt.Allocate();
        var modelDef = objWithProperties.GetType().GetModelDefinition();
        var updateAllFields = updateFields == null || updateFields.Count == 0;

        foreach (var fieldDef in modelDef.FieldDefinitions)
        {
            if (fieldDef.ShouldSkipUpdate())
                continue;

            try
            {
                if (fieldDef.IsPrimaryKey && updateAllFields)
                {
                    if (sqlFilter.Length > 0)
                        sqlFilter.Append(" AND ");

                    sqlFilter
                        .Append(GetQuotedColumnName(fieldDef.FieldName))
                        .Append("=")
                        .Append(this.AddParam(dbCmd, fieldDef.GetValue(objWithProperties), fieldDef).ParameterName);

                    continue;
                }

                if (!updateAllFields && !updateFields.Contains(fieldDef.Name, StringComparer.OrdinalIgnoreCase) || fieldDef.AutoIncrement)
                    continue;

                if (sql.Length > 0)
                    sql.Append(", ");

                // Set parameter based on value - will use named parameters with the value from the object passed
                var parameter = this.AddParameter(dbCmd, fieldDef);
                sql
                    .Append(GetQuotedColumnName(fieldDef.FieldName))
                    .Append("=")
                    .Append(parameter.ParameterName);
                SetParameterValue<object>(fieldDef, parameter, objWithProperties);


                // Current code: adds numbered - non named - parameter with the value from the object passed through the DialectProvider
                //sql
                //    .Append(GetQuotedColumnName(fieldDef.FieldName))
                //    .Append("=")
                //    .Append(this.AddParam(dbCmd, fieldDef.GetValue(objWithProperties), fieldDef).ParameterName);
            }
            catch (Exception ex)
            {
                OrmLiteUtils.HandleException(ex, "ERROR in ToUpdateRowStatement(): " + ex.Message);
            }
        }

        var strFilter = StringBuilderCacheAlt.ReturnAndFree(sqlFilter);
        dbCmd.CommandText = $"UPDATE {GetQuotedTableName(modelDef)} " +
                            $"SET {StringBuilderCache.ReturnAndFree(sql)}{(strFilter.Length > 0 ? " WHERE " + strFilter : "")}";

        if (sql.Length == 0)
            throw new Exception("No valid update properties provided (e.g. p => p.FirstName): " + dbCmd.CommandText);
    }

The change incurs using named parameters by callibng AddParameter instead of AddParam. AddParam adds sequential parameters with values passed though the DialectProvider, whereas AddParameter and SetParameterValue just set the vaklue from the object.

These aren’t really fixes, the premise that GetParamValue() should only be used for specific operations so their overrides only applies to those specific operations is what’s causing these coding backflips to change internal impls to use other APIs. The APIs themselves aren’t designed for specific usage otherwise their purpose would be clear in their name, I want to avoid having “sacred” APIs that can only be used for certain situations, trying to apply these limitations on existing APIs is poor UX that’s going to cause more issues in future.

IMO resolving this should be closer to what’s causing it, so instead of overriding GetParamValue() it’s better off to move your param customization in a filter that applies to select/where or insert/update params.

So I’ve added new InitQueryParam() and InitUpdateParam() APIs to Dialect provider in this commit, where you should be able to move your logic to override the param value in your custom InitQueryParam().

I’ve also changed the existing uses of AddParam to use either AddQueryParam() (which calls InitQueryParam) or AddUpdateParam() (which calls InitUpdateParam), feel free to send a PR if I’ve missed any.

2 Likes

Funny. As you mentioned ‘these are not really fixes’ and that;s exactly why I mentioned that I am not completely happy with it. My preference would be a way to differentiate Update and Query params and that is exactly what you did :smile:

Will test and file a PR when necessary, but as I see it now, this perfectly fits our needs. KUDOS!

Last question: Can you put this on myget pls?

It’s already on MyGet, if you already have v5.4.1 installed you’ll need to clear your NuGet packages cache.

1 Like