Repodb: Enhancement: Allow open/closing quotes to be empty string

Created on 8 Jul 2020  路  9Comments  路  Source: mikependon/RepoDB

We are trialling RepoDb as a client for postgres databases. To avoid the issue with case sensitivity, we have overridden the db settings and set the OpeningQuote and ClosingQuote to empty string. However, we are now experiencing an issue with a Replace call not allowing empty strings.

I have tracked it down to the AsUnquotedInternal method. Looks like the following block need to check the quote properties for empty and return the value if it is.

internal static string AsUnquotedInternal(this string value,
    bool trim,
    IDbSetting dbSetting)
{
    if (dbSetting == null)
    {
        return value;
    }
    value = value
        .Replace(dbSetting.OpeningQuote, string.Empty)
        .Replace(dbSetting.ClosingQuote, string.Empty);
    if (trim)
    {
        value = value.Trim();
    }
    return value;
}
Fixed deployed enhancement request

All 9 comments

Thank you for this issue report. Would you be kind enough to issue a PR here? I will do test the fixes with all the integration tests needed on my own environment.

I have created a pull request that adds a check for empty quotes.

Hi Nick, I need more information before I merge this PR #468. Would you be able to share your class model and how did you made the mapping? Thanks

Hi,

We are looking to target postgres with RepoDb, and don't have any real models in place atm. You may or may not know about postgres's quote hell, but I will elaborate on our scenario in case of the latter.

Postgres has issues with case sensitivity when sql statements involve quotes that are not used consistently. When we create our tables, columns, functions, etc. we don't quote anything and postgres will automatically convert to lowercase. We want to avoid using quotes to simplify our development efforts.

When we use RepoDb it is automatically adding quotes which results in our C# properties not matching our postgres schema (i.e. entity property Id != postgres column id). We could lowercase our entities to match postgres, but want to keep as much best practice in place as possible (i.e. upper camel casing).

So our approach was to override the DbSettings for the postgres connection and set the quote properties to empty string, which would hopefully remove the quotes from the sql statements. However, when we did this, the AsUnquotedInternal method threw an exception because the first argument of Replace is an empty string.

This PR is to put a guard around the Replace so it is not called if the quote values are empty string.

Hope that made sense.

@nickdodd79 great, thank you for this explanation. This is enough for us to accept the PR. We will give you the beta release soon after the necessary Tests were written on top of your PR. We will do it ASAP.

The PR must have been done in the RepoDb.Core line. You can see it here. Anyway, I will transfer it there but I will use the string.IsNullOrEmpty() instead of string.IsNullOrWhitespace().

Btw, did you know that you can also use the Implicit Mapping feature with the use of FluentMapper class? You can pre-set all the mappings in lower-case in a fluent manner, no need to decorate the models with attribute. But the you have to create a method that explore your namespace models and auto-assign it using the FluentMapper.

Though the fix will be available on the next release. But here, I created a dynamic mapping lowerizer named EntityMappingLowerizer for your case. Please see the code from this gist. You can always use this gist for whatever project you have.

How to use?

Via Entity Level:

EntityMappingLowerizer.ForEntity<Customer>();
var tableName = ClassMapper.Get<Customer>(); // Test the model Customer
var columnName = PropertyMapper.Get<Customer>(e => e.Name); // Test the model Customer.Name

Via Namespace (All models on the namespace):

EntityMappingLowerizer.ForNamespace(typeof(Customer).Namespace);
var tableName = ClassMapper.Get<Customer>(); // Test the model Customer
var columnName = PropertyMapper.Get<Customer>(e => e.Name); // Test the model Customer.Name
tableName = ClassMapper.Get<Person>(); // Test the model Person

Please feel free to revert if this has not solved your problem.

@nickdodd79 - would you be able to test this solution using the latest version v1.11.5?

Hi @mikependon

We have upgraded to v1.11.5 and everything seems to be all good. Thanks for the quick turn around in releasing the fix. Much appreciated.

Nick.

Was this page helpful?
0 / 5 - 0 ratings