Postgres primary key logic disabled, needs to be re-enabled

[warmerdam]

Mateusz,

The automatic recognition of primary keys as FIDs in the PG driver is currently (trunk and 1.4.x) broken and this makes it impossible to do some operations on tables (such as DeleteFeature?()) that weren't created by OGR (thus using the recognisable ogc_fid name as fid).

    /* TODO make changes corresponded to Frank issues
       
    sprintf ( szCommand,
              "SELECT a.attname "
              "FROM pg_attribute a, pg_constraint c, pg_class cl "
              "WHERE c.contype='p' AND c.conrelid=cl.oid "
              "AND a.attnum = c.conkey[1] AND a.attrelid=cl.oid "
              "AND cl.relname = '%s'",
              pszTableIn );

    hResult = PQexec(hPGConn, szCommand );

    if ( hResult && PQntuples( hResult ) == 1 && PQgetisnull( hResult,0,0 ) == false )
    {
        sprintf( szPrimaryKey, "%s", PQgetvalue(hResult,0,0) );
        CPLDebug( "OGR_PG", "Primary key name (FID): %s", szPrimaryKey );
    }
    else
    {
        CPLError( CE_Failure, CPLE_AppDefined,
                  "%s", PQerrorMessage(hPGConn) );
        CPLError( CE_Warning, CPLE_AppDefined,
                  "Unable to detect table primary key. Use default 'ogc_fid'");
    }*/

I don't know what Frank issues the comment refers to off hand, though it could be that we need to confirm the primary key is an integer field. Once this logic has been appropriately reenable we need to add a test to the postgres testsuite and ensure it is operational on at least one buildbot platform (likely telascience-full).

[mloskot]

Frank,

Could you refer to the comment:

I don't know what Frank issues the comment refers to off hand(...) ?

[warmerdam]

The code is currently commented out with this message:

    /* TODO make changes corresponded to Frank issues

I don't know what Frank issues refers to. Was it you that commented out the code with this remark?

[mloskot]

Frank, yes I'm referring to this comment but I'm not the author of it and also I didn't commented this code. I tried to find author using Trac but seems it's very old and not available in Trac. Usually, I mark my comments with mloskot.

Anyway, I don't know what issues are referred here :-(

[mloskot]

The comment was made by Oleg Semykin and thanks to Andrey we've managed get some clarifications about his notes. Here are Oleg's comments forwarded by Andrey:

There should be a code to determine whether we have a single field as a primary key. If so the commented out code will work. Sometimes we can have more than one field as a key and there should be some more code to handle this case. And that was a FrankW concern.

and Frank's additional comments about multi-column PK support, in future:

I don't see how we can ever support multi-field primary keys without major changes to OGR. We just have to recognise when the pk will not map to our 32bit integer fid, and fallback to assigning fids.

[mloskot]

I've fixed this issue (r12501) and currently single-column Primary Key should be detected and mapped to FID correctly. If multi-column PK is detected, PG driver will map virtual ogc_fid to feature ID values.

[mloskot]

One additional note, Primary Key is detected as supported and mapped to FID values if both assertions are true:

1. PK for a table consists of single column only

2. Data type of column used as single-column PK is of one of the following types: smallint (16-bit), integer (32-bit) or serial (32-bit) (see  Integer Types in PostgreSQL manual)

Also, if multi-column PK is detected, appropriate warning is printed (point2 is name of table used in this example):

Warning 1: Multi-column primary key detected but not supported.
Warning 1: Unable to detect single-column primary key for 'point2'. Use default 'ogc_fid'

I hope it makes sense.

[mloskot]

The fix has been ported to the branches/1.4 (r12510)

[warmerdam]

Mateusz,

We need to remove the warning issued when there is no primary key:

Warning 1: Unable to detect single-column primary key for
'Gebaeudeadressen__Strassenstueck'. Use default 'ogc_fid'

This generates huge amounts of confusing noise when accessing substantial databases or databases loaded with OGR.

[mloskot]

Fixed in trunk (r12650)

[ajolma]

The current fix r12501 uses ANY(pg_index.indkey) where indkey is int2vector. Support for ANY on int2vector is very recent in PostgreSQL. It is not in 8.0 but is in 8.2. Would there be any other way to implement this functionality than depending on such new feature?

Ari

[warmerdam]

It is very desirable for the PG driver to work with reasonably old Postgres' - at least back to 8.0, if not back into the 7.x'es (even if with moderately reduced functionality). Reopening accordingly.

[ajolma]

According to this:  http://archives.postgresql.org/pgsql-sql/2005-08/msg00148.php there is no nice SQL solution, but there is a solution.

[mloskot]

Applied patch to the trunk (r12931) and branches/1.4 (r12932)