GNOME Bugzilla – Bug 513543
incorrect query to check for primary key
Last modified: 2008-03-25 08:29:16 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.
Could we have a patch for this?
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.
This doesn't seem to apply to either the libgda-3-0 branch or svn trunk. Could you check, please.
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.
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.
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.
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.
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.
So that change should be in libgda-3.0.2 already. Isn't it?
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.
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.
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.
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.