After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 513543 - incorrect query to check for primary key
incorrect query to check for primary key
Status: RESOLVED FIXED
Product: libgda
Classification: Other
Component: PostgreSQL provider
3.1.x
Other Linux
: Normal normal
: ---
Assigned To: malerba
gnome-db Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-01-31 21:44 UTC by Mark Johnson
Modified: 2008-03-25 08:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for query (552 bytes, patch)
2008-03-02 15:39 UTC, Mark Johnson
committed Details | Review

Description Mark Johnson 2008-01-31 21:44:51 UTC
From a discussion on the gnome-db mailing list regarding this query issued by the PostgreSQL provider:
SELECT 1 FROM pg_catalog.pg_class c,
pg_catalog.pg_constraint c2, pg_catalog.pg_attribute a WHERE c.relname =
'slots' AND c.oid = c2.conrelid and a.attrelid = c.oid and c2.contype =
'P' and c2.conkey[1] = a.attnum and a.attname = 'slot_id';

Vivien Malerba posted this:
It seems the "SELECT 1..." query is sent whenever
gda_data_model_describe_column() is called, and it can be optimized to
be called only once...

Mark Johnson:
> But what information does it provide?  Here is a sample session from psql:
> >> issue001_db=# \d+ slots
> >>                          Table "public.slots"
> >>       Column       |          Type          | Modifiers | Description
> >> -------------------+------------------------+-----------+-------------
> >>  slot_id           | integer                | not null  |
> >>  obj_guid          | character(32)          | not null  |
> >>  name              | character varying(500) | not null  |
> >>  slot_type         | integer                | not null  |
> >>  int64_val         | bigint                 |           |
> >>  string_val        | character varying(500) |           |
> >>  double_val        | bigint                 |           |
> >>  timespec_val      | date                   |           |
> >>  guid_val          | character(32)          |           |
> >>  numeric_val_num   | bigint                 |           |
> >>  numeric_val_denom | bigint                 |           |
> >> Indexes:
> >>     "slots_pkey" PRIMARY KEY, btree (slot_id)
> >> Has OIDs: no
> >>
> >> issue001_db=# SELECT 1 FROM pg_catalog.pg_class c,
> >> pg_catalog.pg_constraint c2, pg_catalog.pg_attribute a WHERE c.relname =
> >> 'slots' AND c.oid = c2.conrelid and a.attrelid = c.oid and c2.contype =
> >> 'P' and c2.conkey[1] = a.attnum and a.attname = 'slot_id';
> >>  ?column?
> >> ----------
> >> (0 rows)
> >>
> >> There is not a single row returned by this query.  'slot-id' is the
> >> primary key column, which seems to be something this query might be
> >> looking for.  Does this possibly behave differently in an older version
> >> of PostgreSQL?  I am using 8.2.6.
> >>     
>
> I believe the information is in the existence of a row returned or no.
>   
The c2.contype='P' test checks if the column is a primary key.  This 
alternates with checking the columns for a unique key ('u').  However, 
the query is returning zero rows regardless of the presence or absence 
of a primary or unique key.  In the above instance, the column is a 
primary key as shown by the \d+ command.  When I tried columns that are 
not primary keys, they return the same results.  I believe, therefore, 
that there is a problem with this query.

I have found the problem.  The PostgreSQL manual (version 8.2.6; section 43.13 pg_constraint) lists the meaning of values in the contype column.  However, it shows a lower-case 'p' for primary key.

Here is a sample run from psql on the gnucash db backend database:
gnucash_qif_db=# select conname, contype, conkey[1] from pg_catalog.pg_constraint where contype='p';
        conname        | contype | conkey
-----------------------+---------+--------
 customers_pkey        | p       |      1
 jobs_pkey             | p       |      1
 transactions_pkey     | p       |      1
 splits_pkey           | p       |      1
 lots_pkey             | p       |      1
 budgets_pkey          | p       |      1
 taxtables_pkey        | p       |      1
 taxtable_entries_pkey | p       |      1
 recurrences_pkey      | p       |      1
 employees_pkey        | p       |      1
 orders_pkey           | p       |      1
 commodities_pkey      | p       |      1
 accounts_pkey         | p       |      1
 slots_pkey            | p       |      1
 schedxactions_pkey    | p       |      1
 invoices_pkey         | p       |      1
 prices_pkey           | p       |      1
 entries_pkey          | p       |      1
 vendors_pkey          | p       |      1
 books_pkey            | p       |      1
 billterms_pkey        | p       |      1
(21 rows)

gnucash_qif_db=# select 1 from pg_catalog.pg_class c, pg_catalog.pg_constraint c2, pg_catalog.pg_attribute a
gnucash_qif_db-# where c.relname='slots' and c.oid = c2.conrelid and a.attrelid = c.oid and c2.contype='P' and c2.conkey[1] = a.attnum and a.attname='slot_id';
 ?column?
----------
(0 rows)

gnucash_qif_db=# select 1 from pg_catalog.pg_class c, pg_catalog.pg_constraint c2, pg_catalog.pg_attribute a
where c.relname='slots' and c.oid = c2.conrelid and a.attrelid = c.oid and c2.contype='p' and c2.conkey[1] = a.attnum and a.attname='slot_id';
 ?column?
----------
        1
(1 row)

Note that the only difference between the two queries is the latter uses a lower-case 'p'.  It successfully detects the primary key on that column.

I'm not sure what the functional implications of this are.  I found the query by logging what the PostgreSQL provider sends to the db, and these queries are very common.  I checked into what it was looking for.
Comment 1 Murray Cumming 2008-03-01 18:07:35 UTC
Could we have a patch for this?
Comment 2 Mark Johnson 2008-03-02 15:39:18 UTC
Created attachment 106393 [details] [review]
patch for query

This changes the 'P' to a 'p'.  The query will now return the single row containing '1' when the queried column is a primary key.
Comment 3 Murray Cumming 2008-03-14 07:47:03 UTC
This doesn't seem to apply to either the libgda-3-0 branch or svn trunk. Could you check, please.
Comment 4 malerba 2008-03-14 07:54:15 UTC
I propose to put this on hold until the new implementation of the PostgreSQL provider is finished because it is probable this bug will become obsolete.
Comment 5 Murray Cumming 2008-03-14 09:14:32 UTC
No, I'd really like this simple fix in the libgda-3.0 API. That's the one that's already in distributions, and is already released and stable, so it's the one that will be used for the next year or so.
Comment 6 Mark Johnson 2008-03-15 14:23:58 UTC
Sorry, I should have recorded which revision of which branch I made it against.  The patch was made against revision 3066 of svn trunk.

It also applies against version 3.1.2.  For some reason, I had to cd into the providers directory and pass -p1 to patch.  Otherwise, it claimed to be unable to find the file.  I don't know why.
Comment 7 Murray Cumming 2008-03-15 16:22:13 UTC
This really doesn't apply to the current code in either branch, and I don't see where a similar change should be made now. This seems very important, so I'd really like it if you could try this with the latest 3.0.x version, ideally from svn.
Comment 8 Mark Johnson 2008-03-20 15:57:38 UTC
The check_constraint function and calls to it have been moved to gda-postgres-recordset.c.  I see that the call for the primary key now uses a lower-case 'p'.  Therefore, this is fixed in this branch.

I am looking at revision 3064 of svn release-3-0-branch.
Comment 9 Murray Cumming 2008-03-20 18:43:24 UTC
So that change should be in libgda-3.0.2 already. Isn't it?
Comment 10 Mark Johnson 2008-03-21 02:42:13 UTC
The version reported in this bug was 3.1.x.  I found it in version 3.1.2.  So I believe that this was introduced after 3.0.x.  I don't think it was in 3.0.2 at all.

Should I have been looking at a different branch in svn?

The function check_constraint is not present in V4-branch.  This function is still present in trunk rev 3091.  However, the 'p' is properly lower case.  Therefore, this bug is not in trunk.
Comment 11 Murray Cumming 2008-03-21 09:02:26 UTC
Oh, I don't think anyone branched for 3.1 before changing it to 4. Maybe I can do that retrospectively and release a new 3.1 tarball. Luckily, I doubt that any distros have shipped 3.1.
Comment 12 Murray Cumming 2008-03-25 07:55:03 UTC
I created a libgda-3-2 branch and committed this patch. Please do patch the ChangeLog in future. Thanks for all your effort and sorry for my confusion.

I will also release a new libgda 3.1.x tarball, though I don't think many people are using 3.1 and it is not likely to ever become a stable 3.2.0.
Comment 13 malerba 2008-03-25 08:29:16 UTC
The initial idea was to have a 3.2, but the V4 came to maturity much quicker than I had anticipated so I prefered to break the API/ABI earlier than to do a 3.2 and delay that for a year.

So, I don't plan to invest time into a 3.2.