Re: [PgFoundry] Unsigned Data Types [1 of 2]

Hello Jamie and Tom.

Thank you very much for the feedback and reviews. I will attempt to
answer all the questions I found in this thread in this one email. If I
miss any questions, let me know and I will answer it :)

Jamie: Thanks for the feedback on missing comments. I will go back
and add more comments to the code.

Jamie: Thanks for the patches. I have applied the Makefile patch to my
local tree. I am still reviewing the regressions.diffs patch. I definitely
see the issue now, I am still reviewing to see/understand if your solution
is the proper fix. I am reviewing the main PostgreSQL regression tests
to understand how the COPY is handled.

Jamie: This patch is targeted for as a PGFoundry module. I wanted it
reviewed by the PostgreSQL community for two reasons:
1. To make the data type is correct (i.e. the bugs you and Tom
identified, etc).
2. To go through the community review process so other people can
have faith/trust the module is correct.

After the community is happy with the uint data type, I will commit it to
the PGFoundry repository.

Jamie: The uint1 is an unsigned 8-bit value. It is the unsigned variant
of the "char" type. The uint8 type (which I did not provide support for in
this patch would be the unsigned variant of the int8 (64-bit type).

Jamie and Tom: Tom, you were correct. The -255::uint1 is being promoted
to the int4 data type. Here is the sample c program I used to verify this:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

#include <postgresql/libpq-fe.h>

int main()
{
PGconn *conn;
PGresult *res;
char query[255];

conn = PQconnectdb("host=127.0.0.1 dbname=test user=rbrad");
assert(PQstatus(conn) == CONNECTION_OK);

res = PQexec(conn, "SELECT -255::uint1;");
assert(PQresultStatus(res) == PGRES_TUPLES_OK);

snprintf(query, 255, "SELECT %d::regtype", PQftype(res, 0));
PQclear(res);

res = PQexec(conn, query);
assert(PQresultStatus(res) == PGRES_TUPLES_OK);
printf("Result Type: %s\n", PQgetvalue(res, 0, 0));
PQclear(res);

PQfinish(conn);
return 0;
}

Output:
Result Type: integer

On Sun, Sep 7, 2008 at 9:07 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Ah. The scalarltsel/scalargtsel stuff has always been a bit bogus for
> cross-type comparisons; it assumes it will know both or neither of the
> two datatypes. So you can get away with using those functions for
> uint > uint (although they won't be very bright about it); but using
> them for uint > int fails outright.

> If you read the comments around that stuff it leaves quite a lot to be
> desired, but I don't really have better ideas at the moment. The best
> near-term solution for the uint module is probably not to rely on
> scalarltsel/scalargtsel for uint comparisons, but to make its own
> selectivity functions that know the uint types plus whatever standard
> types you want to have comparisons with.

Ok. Looks like I need to review these functions and develop new functions
specific for the unsigned type. I will work on this tomorrow night and
submit an updated patch.

Thanks again for the feedback and reviews!

- Ryan