Re: [PgFoundry] Unsigned Data Types [1 of 2] - Mailing list pgsql-patches

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@sss.pgh.pa.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