Re: INTERVAL data type and libpq - what format?

Hello,

I try to use the new 8.4 INTERVAL type with libpq, but get crazy with
the input formatting rules...

I use PQprepare() / PQexecPrepared() with parameter list, binding the
INTERVAL values with the 1186 pg_type and passing a string buffer with
values like:

"12345" for an INTERVAL YEAR

The INSERT works without error, but when selecting rows from the table
in psql, I get "00:00:00" values ?!?!

When inserting the value "12345" from the psql command tool it works...

I must be doing something wrong, but I could not find any documentation
on using INTERVAL in libpq...

Can someone from the hackers just tell me if it's supposed to work and
if yes what format is expected by the client library?

Attached, you have a test case to reproduce the problem.

Thanks a lot!
Seb

On Tue, May 19, 2009 at 10:08:37AM +0200, Sebastien FLAESCH wrote:
> I try to use the new 8.4 INTERVAL type with libpq, but get crazy with
> the input formatting rules...

I think you're giving the database conflicting instructions and it's
getting confused.

> fprintf(stdout,"++ Preparing INSERT ...\n");
> paramTypes[0] = 23; /* INT4 */
> paramTypes[1] = 1186; /* INTERVAL */
> paramTypes[2] = 1186; /* INTERVAL */

I don't really know 8.4, but I believe you're saying here that you
explicitly want the values to be of basic INTERVAL type here, i.e. not
INTERVAL DAY TO HOUR for parameter 3.

Thus when you do:

> paramValues[0] = "1";
> paramValues[1] = "-12345 years";
> paramValues[2] = " 123 11:00";
> r = PQexecPrepared(c, "s1", 3, paramValues, NULL, NULL, 0);

It's interpreting " 123 11:00" correctly as a basic INTERVAL value and
then casting it to your more constrained version as you're saving in the
table.

However, when you do:

> paramValues[0] = "2";
> paramValues[1] = "-12345";
> paramValues[2] = " 123 11";
> r = PQexecPrepared(c, "s1", 3, paramValues, NULL, NULL, 0);

You get an error because " 123 11" isn't a valid literal of an
(undecorated) INTERVAL type. I think PG may do the right thing if you
don't specify the types when preparing the query, but haven't tested.

--
Sam http://samason.me.uk/

Yes, good point.

I realize now that I would have expected libpq to give me a way to specify
the exact decoration or precision of INTERVAL parameters...

As you can do with ODBC's SQLBindParameter(), where you specify the C type,
SQL type, precision/scale or length ...
I believe this is important when it comes to data type conversion (for ex,
when you want to insert a numeric/date/time into a char or the other way).
=> sort of cast, actually...

I known libpq functions like PQexecParams() or PQexecPrepared() have a
paramFormats[] parameter to specify if the buffer will hold a string
or the binary representation of the value... but that would not help
much (I don't want to deal with internal structures!).

I can manage to bind only basic INTERVAL values for all sort of INTERVAL
columns, no problem...
I did further tests using the "PnnnYnnnM ..." ISO format and that is
working much better.

However I would expect at least 2 classes of INTERVALs to be specified
in libpq parameters:

INTERVAL YEAR TO MONTH
INTERVAL DAY TO SECOND(n)

Also: I still have the overflow issue with types like INTERVAL SECOND.
=> discussed in another thread "INTERVAL SECOND limited to 59 seconds?"

Thanks a lot!
Seb

Sam Mason wrote:
> On Tue, May 19, 2009 at 10:08:37AM +0200, Sebastien FLAESCH wrote:
>> I try to use the new 8.4 INTERVAL type with libpq, but get crazy with
>> the input formatting rules...
>
> I think you're giving the database conflicting instructions and it's
> getting confused.
>
>> fprintf(stdout,"++ Preparing INSERT ...\n");
>> paramTypes[0] = 23; /* INT4 */
>> paramTypes[1] = 1186; /* INTERVAL */
>> paramTypes[2] = 1186; /* INTERVAL */
>
> I don't really know 8.4, but I believe you're saying here that you
> explicitly want the values to be of basic INTERVAL type here, i.e. not
> INTERVAL DAY TO HOUR for parameter 3.
>
> Thus when you do:
>
>> paramValues[0] = "1";
>> paramValues[1] = "-12345 years";
>> paramValues[2] = " 123 11:00";
>> r = PQexecPrepared(c, "s1", 3, paramValues, NULL, NULL, 0);
>
> It's interpreting " 123 11:00" correctly as a basic INTERVAL value and
> then casting it to your more constrained version as you're saving in the
> table.
>
> However, when you do:
>
>> paramValues[0] = "2";
>> paramValues[1] = "-12345";
>> paramValues[2] = " 123 11";
>> r = PQexecPrepared(c, "s1", 3, paramValues, NULL, NULL, 0);
>
> You get an error because " 123 11" isn't a valid literal of an
> (undecorated) INTERVAL type. I think PG may do the right thing if you
> don't specify the types when preparing the query, but haven't tested.
>
>

Sam Mason <sam(at)samason(dot)me(dot)uk> writes:
> I don't really know 8.4, but I believe you're saying here that you
> explicitly want the values to be of basic INTERVAL type here, i.e. not
> INTERVAL DAY TO HOUR for parameter 3.

Right, you can get the equivalent behavior from psql thus:

regression=# select '-12345'::interval::interval year;
interval
----------
00:00:00
(1 row)

regression=# select '12 11'::interval::interval year;
ERROR: invalid input syntax for type interval: "12 11"
LINE 1: select '12 11'::interval::interval year;
^

There is not any way to bind a more specific type to a parameter at the
protocol level.

> I think PG may do the right thing if you
> don't specify the types when preparing the query, but haven't tested.

Yeah, that should work (though I haven't verified it either). Another
common trick is to specify the type in the text of the query by casting
the parameter symbol:

PQprepare( ... $2::INTERVAL YEAR ... );

I'd say this is better style than hard-wiring numeric type OIDs into
your code.

regards, tom lane

On Tue, May 19, 2009 at 02:17:20PM +0200, Sebastien FLAESCH wrote:
> As you can do with ODBC's SQLBindParameter(), where you specify the C type,
> SQL type, precision/scale or length ...
> I believe this is important when it comes to data type conversion (for ex,
> when you want to insert a numeric/date/time into a char or the other way).
> => sort of cast, actually...

Tom sent a message, but it seems to have got lost somewhere. The
suggestion was to leave the paramTypes empty and just write the prepared
statement as:

INSERT INTO tbl (k,i1,i2) VALUES ($1::INT,$2::INTERVAL YEAR,$3::INTERVAL);

or similar. That way PG will be able to infer that $1 will be a literal
of integer type, $2 will be of INTERVAL YEAR and so on. In fact for
queries such as this I don't think you even need to put those casts in
there as PG will be able to figure out what you mean automatically (i.e.
it does a limited form of type inference).

> I known libpq functions like PQexecParams() or PQexecPrepared() have a
> paramFormats[] parameter to specify if the buffer will hold a string
> or the binary representation of the value... but that would not help
> much (I don't want to deal with internal structures!).

Yes, stay away from binary types if at all possible!

> Also: I still have the overflow issue with types like INTERVAL SECOND.
> => discussed in another thread "INTERVAL SECOND limited to 59 seconds?"

If I read it correctly; it's not overflow but truncation. In PG (I've
got no idea what the database you're comparing to does/what the spec
says it's supposed to do) when you you want a value of type "INTERVAL
SECOND" then PG is interpreting this as meaning I want a value of type
INTERVAL where all the fields apart from the seconds are zero. Whether
this is useful seems debatable, Richard's suggestion of creating a set
of custom types that do the right thing for you seems like the path of
least resistance.

--
Sam http://samason.me.uk/

On Tue, May 19, 2009 at 8:17 AM, Sebastien FLAESCH <sf(at)4js(dot)com> wrote:
> Yes, good point.
>
> I realize now that I would have expected libpq to give me a way to specify
> the exact decoration or precision of INTERVAL parameters...
>
> As you can do with ODBC's SQLBindParameter(), where you specify the C type,
> SQL type, precision/scale or length ...
> I believe this is important when it comes to data type conversion (for ex,
> when you want to insert a numeric/date/time into a char or the other way).
> => sort of cast, actually...
>
> I known libpq functions like PQexecParams() or PQexecPrepared() have a
> paramFormats[] parameter to specify if the buffer will hold a string
> or the binary representation of the value... but that would not help
> much (I don't want to deal with internal structures!).

You might want to take a look at 'libpqtypes'. It exposes the
internal formats binary formats in easy to use structures.

e.g. (in libpqtypes.h)
typedef struct
{
int years;
int mons;
int days;
int hours;
int mins;
int secs;
int usecs;
} PGinterval;

I was curious, and decided to see what happens when you inserted an
interval with the following code snippet:

PGinterval i;

memset(&i, 0, sizeof(i));
i.secs = 1000000;

PQputf(p, "%interval", &i);
PQparamExec(c, p, "insert into foo values ($1)", 0);

select * from foo;
i
-----------
277:46:40

also, libpqtypes always sends in binary which is much faster for the
date/time types.

http://libpqtypes.esilo.com/

merlin

On Tue, May 19, 2009 at 10:12 AM, Sam Mason <sam(at)samason(dot)me(dot)uk> wrote:
> On Tue, May 19, 2009 at 02:17:20PM +0200, Sebastien FLAESCH wrote:
>> As you can do with ODBC's SQLBindParameter(), where you specify the C type,
>> SQL type, precision/scale or length ...
>> I believe this is important when it comes to data type conversion (for ex,
>> when you want to insert a numeric/date/time into a char or the other way).
>> => sort of cast, actually...
>
> Tom sent a message, but it seems to have got lost somewhere.  The
> suggestion was to leave the paramTypes empty and just write the prepared
> statement as:
>
>  INSERT INTO tbl (k,i1,i2) VALUES ($1::INT,$2::INTERVAL YEAR,$3::INTERVAL);
>
> or similar.  That way PG will be able to infer that $1 will be a literal
> of integer type, $2 will be of INTERVAL YEAR and so on.  In fact for
> queries such as this I don't think you even need to put those casts in
> there as PG will be able to figure out what you mean automatically (i.e.
> it does a limited form of type inference).
>
>> I known libpq functions like PQexecParams() or PQexecPrepared() have a
>> paramFormats[] parameter to specify if the buffer will hold a string
>> or the binary representation of the value... but that would not help
>> much (I don't want to deal with internal structures!).
>
> Yes, stay away from binary types if at all possible!
>

For the record, I disagree with this sentiment strongly. I would
rather see the client side library be buffed up so you have an
opportunity to deal with the data structures the way the server sees
them. The more complex the type is (like date time types), the
bigger the win both in terms of performance and feature exposure. I
understand though that cross database support is impossible though.

merlin

Thank you guys for your input, I really appreciate.

It's a while I haven't posted on this list and be happy to get fast and
accurate answers...

As I wrote in a previous mail, I maintain a database driver for our 4GL runtime
system, allowing Informix 4gl applications to use PostgreSQL.

In this context, we have unknown SQL statements to prepare/execute, so it's
note easy to patch the SQL on the fly to add some cast clauses as Sam
suggested...

Regarding your suggestion to use libpqtypes.h:

As a dev tool provider, we cannot force our customers to rely on add-ons
or extensions. Our driver must work with a standard PostgreSQL database.

By the way,

I would also feel more comfortable if the type ids to be passed to the
paramTypes[] array would be provided in a public header file.

I don't understand why this is not published...

Many thanks,
Seb

Merlin Moncure wrote:
> On Tue, May 19, 2009 at 8:17 AM, Sebastien FLAESCH <sf(at)4js(dot)com> wrote:
>> Yes, good point.
>>
>> I realize now that I would have expected libpq to give me a way to specify
>> the exact decoration or precision of INTERVAL parameters...
>>
>> As you can do with ODBC's SQLBindParameter(), where you specify the C type,
>> SQL type, precision/scale or length ...
>> I believe this is important when it comes to data type conversion (for ex,
>> when you want to insert a numeric/date/time into a char or the other way).
>> => sort of cast, actually...
>>
>> I known libpq functions like PQexecParams() or PQexecPrepared() have a
>> paramFormats[] parameter to specify if the buffer will hold a string
>> or the binary representation of the value... but that would not help
>> much (I don't want to deal with internal structures!).
>
> You might want to take a look at 'libpqtypes'. It exposes the
> internal formats binary formats in easy to use structures.
>
> e.g. (in libpqtypes.h)
> typedef struct
> {
> int years;
> int mons;
> int days;
> int hours;
> int mins;
> int secs;
> int usecs;
> } PGinterval;
>
> I was curious, and decided to see what happens when you inserted an
> interval with the following code snippet:
>
> PGinterval i;
>
> memset(&i, 0, sizeof(i));
> i.secs = 1000000;
>
> PQputf(p, "%interval", &i);
> PQparamExec(c, p, "insert into foo values ($1)", 0);
>
> select * from foo;
> i
> -----------
> 277:46:40
>
> also, libpqtypes always sends in binary which is much faster for the
> date/time types.
>
> http://libpqtypes.esilo.com/
>
> merlin
>

Tom Lane wrote:
> Sam Mason <sam(at)samason(dot)me(dot)uk> writes:
>> I don't really know 8.4, but I believe you're saying here that you
>> explicitly want the values to be of basic INTERVAL type here, i.e. not
>> INTERVAL DAY TO HOUR for parameter 3.
>
> Right, you can get the equivalent behavior from psql thus:
>
> regression=# select '-12345'::interval::interval year;
> interval
> ----------
> 00:00:00
> (1 row)
>
> regression=# select '12 11'::interval::interval year;
> ERROR: invalid input syntax for type interval: "12 11"
> LINE 1: select '12 11'::interval::interval year;
> ^
>
> There is not any way to bind a more specific type to a parameter at the
> protocol level.
>
>> I think PG may do the right thing if you
>> don't specify the types when preparing the query, but haven't tested.
>
> Yeah, that should work (though I haven't verified it either). Another
> common trick is to specify the type in the text of the query by casting
> the parameter symbol:
>
> PQprepare( ... $2::INTERVAL YEAR ... );
>
> I'd say this is better style than hard-wiring numeric type OIDs into
> your code.

Remember we are implementing a database driver with equivalent features
and an ODBC driver for PostgreSQL, executing queries with ? parameter
placeholders in the SQL text...

Since SQL Parameter types are not known at (4gl language-level) PREPARE
time, we wait for the (4gl) EXECUTE time to do the real PQprepare() with
paramTypes[]... (this is a pity by the way since we can't get any SQL
error at PREPARE time).

It's not that easy for us to add the ::<type> clauses because the conversion
of the ? placeholders to $n is done at PREPARE time, when types are not yet
yet... so this means major rewriting...

But this is all internal stuff you are not interested in, the main question
I would like to ask is:

What versions of PostgreSQL are 100% sure supporting the $n::<type> clauses?

We have to support all PostgreSQL versions, starting from 8.0 ...

Thanks
Seb

Sam Mason wrote:
> You get an error because " 123 11" isn't a valid literal of an
> (undecorated) INTERVAL type.

Hmm..... should it be?
Skimming the spec makes me think it might be a valid day-time interval.

Quoting the spec:
<unquoted interval string> ::=
[ <sign> ] { <year-month literal> | <day-time literal> }
...
<day-time literal> ::=
<day-time interval>
| <time interval>
<day-time interval> ::=
<days value> [ <space> <hours value> [ <colon> <minutes value>
[ <colon> <seconds value> ] ] ]

I can send a patch if this interpretation is right...

Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
> Sam Mason wrote:
>> You get an error because " 123 11" isn't a valid literal of an
>> (undecorated) INTERVAL type.

> Hmm..... should it be?

Well, we do allow it if it's *explicitly* stated to be a day to hour
interval:

regression=# select interval '123 11' day to hour;
interval
-------------------
123 days 11:00:00
(1 row)

What's at issue here is what should happen without that context.
I'm inclined to think this is ambiguous enough that accepting it
silently isn't such a great idea. I'm also not convinced that the
SQL spec says we must --- the syntax for <interval literal> does
not appear to allow omitting the fields specification.

In a related example,

regression=# select interval '123 11' day;
interval
----------
134 days
(1 row)

we seem to be adding the 123 and 11 together. This is, um,
surprising behavior ... I'd be inclined to think throwing an
error is more appropriate.

regards, tom lane

Tom Lane wrote:
> Ron Mayer <rm_pg(at)cheapcomplexdevices(dot)com> writes:
>> Sam Mason wrote:
>>> You get an error because " 123 11" isn't a valid literal of an
>>> (undecorated) INTERVAL type.
>
>> Hmm..... should it be?
>
> Well, we do allow it if it's *explicitly* stated to be a day to hour
> interval:
>
> regression=# select interval '123 11' day to hour;
> interval
> -------------------
> 123 days 11:00:00
> (1 row)
>
> What's at issue here is what should happen without that context.
> I'm inclined to think this is ambiguous enough that accepting it
> silently isn't such a great idea. I'm also not convinced that the
> SQL spec says we must --- the syntax for <interval literal> does
> not appear to allow omitting the fields specification.
>
> In a related example,
>
> regression=# select interval '123 11' day;
> interval
> ----------
> 134 days
> (1 row)
>
> we seem to be adding the 123 and 11 together. This is, um,
> surprising behavior ... I'd be inclined to think throwing an
> error is more appropriate.
>
> regards, tom lane
>

FYI, with Informix, you don't pass a string in interval literals;
You pass digits directly and the interval qualifiers are mandatory:

=======================================================================

> select interval('123 11') from systables where tabid=1;

201: A syntax error has occurred.
Error in line 1
Near character position 27

> select interval(123 11) from systables where tabid=1;

201: A syntax error has occurred.
Error in line 1
Near character position 25

> select interval(123 11) day to hour from systables where tabid=1;

1261: Too many digits in the first field of datetime or interval.
Error in line 1
Near character position 37

> select interval(123 11) day(3) to hour from systables where tabid=1;

(constant)

123 11

1 row(s) retrieved.

=======================================================================

Seb

I wrote:
> In a related example,

> regression=# select interval '123 11' day;
> interval
> ----------
> 134 days
> (1 row)

> we seem to be adding the 123 and 11 together. This is, um,
> surprising behavior ... I'd be inclined to think throwing an
> error is more appropriate.

I looked into this and found out why the code wasn't throwing an error
already: it seems to be a case of ancient bit-shaving. DecodeInterval
maintains an integer bitmask of field types it's seen already, and
reports an error if a new field's bit is already set in that mask.
However, DAY and WEEK are coded to share a bit, as are YEAR, DECADE,
CENTURY, and MILLENIUM; which means the code can't throw error for
multiple occurrences of any one of those field types. However, there
is room in the mask for all these fields to have their own bits, so
the attached proposed patch just does that.

The potential objections I can see to this patch are:

(1) it eats more than half of the remaining bits in the DTK_M() mask.
This doesn't seem to me to be a big problem --- it's not like people
are likely to come up with many more time intervals for us to support.
Also we could probably shave some bits by eliminating redundancies
if we had to. (For instance, I think IGNORE_DTF could safely be given
a code above 31 since there's no need for it to be in the bitmasks.)

(2) WEEK, DECADE, CENTURY, and MILLENNIUM are too generic to be used
as #define names without some kind of prefix or suffix. This is a real
risk, but considering the other #defines in this list have no prefix or
suffix, it seems inconsistent to put one on these. The buildfarm should
tell us soon enough if there's a real problem (the code does compile
cleanly for me). If we do have a problem, we should add a prefix/suffix
to all these macros at once; but I'd rather not do that as part of a
small bugfix.

Comments?

regards, tom lane