PlainText Sql projection mapping issue

Posts   
 
    
Posts: 1251
Joined: 10-Mar-2006
# Posted on: 05-Oct-2017 21:16:00   

Up until you released the above, we used dapper. As we are trying to get rid of dapper we noticed some pretty strict requirements enforced by your code and I wanted to see if we can get this relaxed.

The POCO we use to project the Sql results on must match the types exactly when using self servicing TypedListDAO().FetchQueryAsync<poco>()

For example, if you have

select SomeIntCol from table

and try and map that to a poco that looks like

public class SomeClass { public string SomeIntCol {get;set;} }

it will fail.

It cannot map the int onto the string. I was wondering if you could instead use Convert or something so these get mapped. Dapper would happily do any of these automatically.

The constraints get worse. If you have this:

select case when MyMoney is null then 0 else MyMoney end as MoneyColumn from table

it will fail on this poco:

public class SomeClass { public decimal MoneyColumn {get;set;} }

because it cannot map the zero that is an int to a decimal! To fix, we have to cast the zero above as money type, etc.

Or if the int on sql server is a smallint, then we have to have short in our poco, etc.

Any possibility these fields values can be converted to match the poco? Maybe just use ConvertTo()?

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39588
Joined: 17-Aug-2003
# Posted on: 06-Oct-2017 11:30:27   

WayneBrantley wrote:

Up until you released the above, we used dapper. As we are trying to get rid of dapper we noticed some pretty strict requirements enforced by your code and I wanted to see if we can get this relaxed.

The POCO we use to project the Sql results on must match the types exactly when using self servicing TypedListDAO().FetchQueryAsync<poco>()

For example, if you have

select SomeIntCol from table

and try and map that to a poco that looks like

public class SomeClass { public string SomeIntCol {get;set;} }

it will fail.

It cannot map the int onto the string. I was wondering if you could instead use Convert or something so these get mapped. Dapper would happily do any of these automatically.

The code uses a wrapper around a datareader to obtain the values. It knows only about the type of the property it has to set, which in this case is a string. It then calls IDataReader.GetString(ordinal) to obtain the value. It can't know it is an int.

To fix this is to make it slower, i.e. by first grabbing the datarow as an object array and use the ProjectionRow class we have and use for typedview/list/query to poco projections which can pull a value from an object array and fall back on Convert.To... The downside is that this is slower and leads to boxing.

The plainsql API is designed to as fast as possible with no conversions: the code consumes the datareader without any conversions other than DBNull->null. We have 3 other query systems which are slower, this one is designed to be as fast as it possibly can.

The constraints get worse. If you have this:

select case when MyMoney is null then 0 else MyMoney end as MoneyColumn from table

it will fail on this poco:

public class SomeClass { public decimal MoneyColumn {get;set;} }

because it cannot map the zero that is an int to a decimal! To fix, we have to cast the zero above as money type, etc.

Why is that strange? You want to store an int in a decimal property. That there's an implicit conversion defined doesn't make it correct. What if you fetch a double and you want to store it in an int property? How to do that conversion, as there are multiple roundings possible and standardized?

Or if the int on sql server is a smallint, then we have to have short in our poco, etc.

and what if you convert an int to a short?

Question of course is, why not make the types fit with what you're fetching. That the types are converted from one type to another doesn't make the value stay the same: you're losing precision in some cases and likely aren't aware of that when the conversions happen automatically.

Any possibility these fields values can be converted to match the poco? Maybe just use ConvertTo()?

We could, with PlainSQLFetchAspects, offer a way to pick the slower path, i.e. with GetValues() -> ProjectionRow usage to convert the objectarray into values, which does allow the fallback to a different type using Convert, so that you can switch to it if you need to. Won't be in v5.3 tho, but we can add that in a later version.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1251
Joined: 10-Mar-2006
# Posted on: 06-Oct-2017 14:04:30   

To fix this is to make it slower

I knew that was coming... simple_smile

and what if you convert an int to a short?

No, I get it, just a short obviously goes into an int, yet the code breaks.

The plainsql API is designed to as fast as possible with no conversions

I can appreciate that, but we do not opt to the plainsql API to gain speed. We use it to gain functionality. Things that cannot be done in the other apis. I would hope that is the only reason other people use it....unless they have some crazy super hot path where the speed increase is critical...and that seems unlikely.

Question of course is, why not make the types fit with what you're fetching.

It is just difficult to do. It means run the code several times (changing parameters and exercising any outer joins, case statements, etc ) to see if the projection crashes. We are having to change our SQL to do casts to data like in the "CASE" statement below. The select may be from a view, that view may have CASE or join, so you have to concern yourself with all those types and cases too. So, nothing is wrong with the sql, except when we try to project it. Not a big fan of having to change SQL so a projection will work. We went through this exercise to change our POCO on what we have converted from dapper that crashed and it was painful. We put a hold on the rest of the conversion as it is pretty painful. Knowing the exact type returned from SQL is not always obvious (CASE statement below) and results in quite a bit of trial and error.

For us we would rather have it just work and not worry about it as opposed to saving a few microseconds, so PlainSQLFetchAspects option would be nice. If it is easily determined what the types are, we would of course map it correctly...and maybe have that turned off....but at the first sign of an issue, we would turn it on!

Thanks for considering.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39588
Joined: 17-Aug-2003
# Posted on: 06-Oct-2017 14:58:03   

WayneBrantley wrote:

To fix this is to make it slower

I knew that was coming... simple_smile

Usually I'm very unpredictable wink

and what if you convert an int to a short?

No, I get it, just a short obviously goes into an int, yet the code breaks.

The plainsql API is designed to as fast as possible with no conversions

I can appreciate that, but we do not opt to the plainsql API to gain speed. We use it to gain functionality. Things that cannot be done in the other apis. I would hope that is the only reason other people use it....unless they have some crazy super hot path where the speed increase is critical...and that seems unlikely.

That's a fair point.

Question of course is, why not make the types fit with what you're fetching.

It is just difficult to do. It means run the code several times (changing parameters and exercising any outer joins, case statements, etc ) to see if the projection crashes. We are having to change our SQL to do casts to data like in the "CASE" statement below. The select may be from a view, that view may have CASE or join, so you have to concern yourself with all those types and cases too. So, nothing is wrong with the sql, except when we try to project it. Not a big fan of having to change SQL so a projection will work. We went through this exercise to change our POCO on what we have converted from dapper that crashed and it was painful. We put a hold on the rest of the conversion as it is pretty painful. Knowing the exact type returned from SQL is not always obvious (CASE statement below) and results in quite a bit of trial and error.

For us we would rather have it just work and not worry about it as opposed to saving a few microseconds, so PlainSQLFetchAspects option would be nice. If it is easily determined what the types are, we would of course map it correctly...and maybe have that turned off....but at the first sign of an issue, we would turn it on! Thanks for considering.

I see your point. You're abit too late tho for v5.3, as everything is finalized already for that release so I can't squeeze it into that anymore, but I've filed a workitem for this for a future release. It's likely not a lot of work (as it just has to flip from one way to project the rows to another) but still has to go through all the phases of making a change.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1251
Joined: 10-Mar-2006
# Posted on: 06-Oct-2017 17:21:57   

That will work, thank you again.

Posts: 1251
Joined: 10-Mar-2006
# Posted on: 06-Feb-2018 06:38:47   

Saw you are alpha on 5.4, hoping this can make it in - so I can convert off of dapper.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39588
Joined: 17-Aug-2003
# Posted on: 06-Feb-2018 10:45:56   

It's on the list of workitems to implement indeed simple_smile We'll likely go for a setting in the projectionsettings object so you have to actively set a setting in that object to make the projection flip to the slower path, as by default we want to use the faster path of course. We might add a static global setting for this as well which is overruled by the setting in the projectionsettings, so you can set it once for the whole app or set it per query.

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1251
Joined: 10-Mar-2006
# Posted on: 06-Feb-2018 16:50:01   

Being able to set it in code globally or 'per query' is a great idea.
Given that - I would not even use the project settings one.

Great news.

Thanks as always!

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39588
Joined: 17-Aug-2003
# Posted on: 08-Feb-2018 11:18:28   

WayneBrantley wrote:

The constraints get worse. If you have this:

select case when MyMoney is null then 0 else MyMoney end as MoneyColumn from table

it will fail on this poco:

public class SomeClass { public decimal MoneyColumn {get;set;} }

because it cannot map the zero that is an int to a decimal! To fix, we have to cast the zero above as money type, etc.

Looking into implementing this, I try to reproduce this, but fail.

Poco:

public class OrderDetailsDifferentType
{
    public int OrderId { get; set; }
    public int ProductId { get; set; }
    public decimal UnitPrice { get; set; }
}

query:


[Test]
public void RetrievalQuery_Names_TypeConversion()
{
    using(var adapter = new DataAccessAdapter())
    {
        var q = @"select orderid, productId, case when unitprice > 10.0 then 0 else unitprice end as UnitPrice from [order details] where orderid=@orderid and productid in (@productids) order by productid asc";
        var result = adapter.FetchQuery<OrderDetailsDifferentType>(q, new { orderid=10248, productids =new[] {11, 42}});
        Assert.AreEqual(10248, result[0].OrderId);
        Assert.AreEqual(2, result.Count);
        Assert.AreEqual(11, result[0].ProductId);
        Assert.AreEqual(0M, result[0].UnitPrice);
        Assert.AreEqual(9.8M, result[1].UnitPrice);
        Assert.AreEqual(42, result[1].ProductId);
    }
}

Test works fine, value is a decimal coming from the DB, not an int, even tho it's converted to 0. Also tried it with a nullable column where I convert the null to 0, same thing: the result is of the type of the column.

e.g.


select case when freight is null then 0 else freight end as FreightCol from orders 
where orderid = 10409

where I have converted the freight column to null for that particular record, results in 0.00 in the resultset, not 0.

Just a FYI, as you mentioned it in the startpost. I understand this is just a part of the problem, where smallint->int and int->string (to name a few) are other parts of the problem. I'll try both as well to see if these can be converted implicitly using Convert. Not every type will work though, if no conversion is defined for it in the default conversion system of .NET you have to take care of it yourself, but as you control the query, that's part of using a low-level system.

(edit) smallint -> int indeed reproduces the problem. That will work for a good test simple_smile

[Test]
public void RetrievalQuery_Names_TypeConversion()
{
    using(var adapter = new DataAccessAdapter())
    {
        var q = @"select orderid, productId, quantity from [order details] where orderid=@orderid order by productid asc";
        var result = adapter.FetchQuery<OrderDetailsDifferentType>(q, new { orderid=10248});
        Assert.AreEqual(10248, result[0].OrderId);
        Assert.AreEqual(3, result.Count);
        Assert.AreEqual(11, result[0].ProductId);
        Assert.AreEqual(12, result[0].Quantity);
    }
}

public class OrderDetailsDifferentType
{
    public int OrderId { get; set; }
    public int ProductId { get; set; }
    public int Quantity { get; set; }
}

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39588
Joined: 17-Aug-2003
# Posted on: 08-Feb-2018 11:53:07   

Ok, both work now simple_smile Implemented in v5.4 (yes this is adapter, selfservicing will have it too, no worries wink )


public class OrderDetailsDifferentType
{
    public int OrderId { get; set; }
    public int ProductId { get; set; }
    public int Quantity { get; set; }
}

public class OrderDetailsAllStrings
{
    public string OrderId { get; set; }
    public string ProductId { get; set; }
    public string Quantity { get; set; }
}

[Test]
public void RetrievalQuery_Names_TypeConversion()
{
    using(var adapter = new DataAccessAdapter())
    {
        var q = @"select orderid, productId, quantity from [order details] where orderid=@orderid order by productid asc";
        var result = adapter.FetchQuery<OrderDetailsDifferentType>(new PlainSQLFetchAspects(performImplicitTypeConversions:true), q, new { orderid=10248});
        Assert.AreEqual(10248, result[0].OrderId);
        Assert.AreEqual(3, result.Count);
        Assert.AreEqual(11, result[0].ProductId);
        Assert.AreEqual(12, result[0].Quantity);
    }
}


[Test]
public void RetrievalQuery_Names_TypeConversion2()
{
    using(var adapter = new DataAccessAdapter())
    {
        var q = @"select orderid, productId, quantity from [order details] where orderid=@orderid order by productid asc";
        var result = adapter.FetchQuery<OrderDetailsAllStrings>(new PlainSQLFetchAspects(performImplicitTypeConversions:true), q, new { orderid=10248});
        Assert.AreEqual("10248", result[0].OrderId);
        Assert.AreEqual(3, result.Count);
        Assert.AreEqual("11", result[0].ProductId);
        Assert.AreEqual("12", result[0].Quantity);
    }
}

for completeness the global version (selfservicing this time). Flag has to be set once.




[Test]
public void RetrievalQuery_Names_TypeConversion4()
{
    var flagSave = PlainSQLFetchAspects.AlwaysPerformImplicitTypeConversions;
    try
    {
        PlainSQLFetchAspects.AlwaysPerformImplicitTypeConversions = true;
        var q = @"select orderid, productId, quantity from [order details] where orderid=@orderid order by productid asc";
        var result = new TypedListDAO().FetchQuery<OrderDetailsAllStrings>(null, q, new { orderid=10248});
        Assert.AreEqual("10248", result[0].OrderId);        // int/short to string conversion
        Assert.AreEqual(3, result.Count);
        Assert.AreEqual("11", result[0].ProductId);
        Assert.AreEqual("12", result[0].Quantity);      
    }
    finally
    {
        PlainSQLFetchAspects.AlwaysPerformImplicitTypeConversions = flagSave;
    }
}

Async is of course supported too, as well as ordered params

Frans Bouma | Lead developer LLBLGen Pro
Posts: 1251
Joined: 10-Mar-2006
# Posted on: 08-Feb-2018 15:39:01   

Great. This will free us up to not need dapper! Thanks

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39588
Joined: 17-Aug-2003
# Posted on: 08-Feb-2018 21:22:12   

Cool! less dapper is always good sunglasses

Frans Bouma | Lead developer LLBLGen Pro