Repository navigation
Not possible to read certain interval values #4599
Description
Steps to reproduce
Run the following code:
// do whatever it takes to open a npgsql connection conn (connect string etc.)
var cmd = conn.CreateCommand();
cmd.CommandText = "SELECT justify_interval('364 days') AS test";
var dr = cmd.ExecuteReader();
while (dr.Read())
{
// Exception occurs here...
object o = dr["test"];
}
System.InvalidCastException: 'Cannot convert interval value with non-zero months to TimeSpan'
The issue
I can't upgrade my software to the latest version of npgsql because of the breaking change #4137 introduced in version 6.0.1.
I do certain calculations in my SQL statements which make it difficult to ensure that postgres returns an interval without any month.
I know it feels wrong that in postgres a month is always 30 days, especially since the statement above returns "1 year 4 days".
But in my opinion it should not be up to the driver to tell me that what postgres is doing does not make a lot sense.
If I can run the statement in pgAdmin I should be able to run it from .NET using npgsql as well.
The old code in the driver was producing exactly the result that was to be expected, a timespan with 364 days 0 hours etc.
If you do not want to revert the code - is it not possible to make this at least a setting in the connect string so users who do not want the exception can have the old behavior back?
With the current driver I would have to write a lot of useless exception handling code for an academic limitation imposed by the driver.
Activity
[-]Not poss[/-][+]Not possible to read certain interval values[/+]roji commentedon Aug 10, 2022
A month isn't always in PostgreSQL; it's a variable unit of time, as it should be (since different months have different number of days). For example:
It's true that
justify_daysandjustify_intervalspecifically treat months as 30-day units; they do this since they have no context as to which month is meant (and so should be used with care!), but PostgreSQL doesn't generally treat months as 30-day units.The point in #4137 was that Npgsql, i.e. the database driver, was making an arbitrary decision to convert months to 30-day units when reading a PG interval into a .NET TimeSpan. This was done at some point since .NET TimeSpan doesn't support months, because it represents an absolute duration of time, measurable in seconds (whereas months are not). It's true that Npgsql was behaving a bit similar to the PG justify_interval function (assume 30 days in the absence of context), but I do not believe a database driver should be making that arbitrary decision for you; this could cause incorrect calculations.
I'd recommend taking a look at the NodaTime date/time library, which Npgsql fully supports (docs). NodaTime has both a Duration type (absolute length of time, measurable in seconds, similar to TimeSpan), and Period, which properly corresponds to PG interval and knows about months as a variable unit (see the section at the bottom of this page). When using the NodaTime plugin, you can seamlessly read a PG interval as a NodaTime Period, and then access the months, perform calculations, anything you want.
If NodaTime isn't an option and you want to keep using TimeSpan, then you'll need to explicitly make the conversion from month->30 days, rather than relying on the driver to implicitly do it for you. You can do this by converting an interval to the total number of seconds, and then reconstructing a day-justified interval from that:
I understand this is a pain and a hurdle to upgrading to 6.0, but the original implicit conversion in Npgsql should never have been done in the first place. I hope the above helps.
Chr-72 commentedon Aug 10, 2022
It still is a bit painful having to rewrite all SQL statements so they work again.
I would also hate to create an extra 3rd party reference to NodaTime which we are not using anywhere else.
Is there no way to read such a value without the extension?
Like you can read date as int and timestamp as long?
I understand that the NpgsqlTimeSpan will disappear in 7.0, so the following would only be a temporary workaround:
Is it possible to read this as struct, byte array or 128 bit integer and parse the data myself?
roji commentedon Aug 10, 2022
Yes - there's NpgsqlInterval, which exactly represents a PG interval, and exposes months, days and time-of-day (as microseconds). You can use that to read your PG interval and then convert it into a TimeSpan, if you want.
roji commentedon Aug 10, 2022
I'll go ahead and close this issue as I don't think we'll be changing anything, but if you still have questions or observations please feel free to continue posting here.
Chr-72 commentedon Aug 10, 2022
Cool, thanks.
I will use the NpgsqlInterval instead then. I assume this will stay in future versions as well unlike NpgsqlTimeSpan.
Would it not be an idea to return this instead of throwing an exception?
So in above example ( object o = dr["test"]; ) o would be a TimeSpan in case months = 0 and a NpgsqlInterval otherwise.
In my opinion this is more user friendly than just saying "can't read it, sorry".
The programmer would notice that is not a normal TimeSpan and has to take appropriate measures himself.
And if he would be casting the result to a timespan he would get an invalid cast exception which is a similar effect as the exception you are now throwing all the time.
roji commentedon Aug 10, 2022
Yes, this type is here to stay; it's simply an exact representation of the data that Npgsql receives from PostgreSQL, nothing more, nothing less.
Npgsql generally doesn't return different .NET types based on the data value it receives... That would make it very difficult for an application to deal with, as Npgsql would sometimes return one type, sometimes another; an application expecting a TimeSpan would suddenly generate a InvalidCastException when it attempts to cast to TimeSpan, which is worse than the exception raised by Npgsql (which at least contains the reason). Though we could improve the exception message to point to NodaTime Period and NpgsqlInterval.
More detail exception message for reading interval with months as Tim…
Better exception message when reading interval with months as TimeSpan
Better exception message when reading interval with months as TimeSpan
roji commentedon Aug 10, 2022
@Chr-72 see #4600 where I've added more detail to the exception message.
Chr-72 commentedon Aug 10, 2022
Thanks. I guess this will point other developers in the right direction.
I thought a different return type would not be such an issue since it might be System.DBNull anyway in case the database has no value. But for me the case is closed since I can avoid exceptions altogether by using the
GetFieldValue<NpgsqlInterval>method instead.Better exception message when reading interval with months as TimeSpan (