Thread: pg_stat_reset() weirdness
Hi guys, If you apply the pg_stat_reset() function patch you get this regression failure. Is this because it's returning a bool I guess? Shall I just fix the regression test to exclude this function? Chris *** ./expected/opr_sanity.out Fri Jul 19 07:11:32 2002 --- ./results/opr_sanity.out Fri Aug 9 13:26:00 2002 *************** *** 27,34 **** AND p1.proname !~ 'costestimate$' AND p1.proname != 'update_pg_pwd_and_pg_group'; oid | proname ! -----+--------- ! (0 rows) -- Look for conflicting proc definitions (same names and input datatypes). -- (This test should be dead code now that wehave the unique index --- 27,35 ---- AND p1.proname !~ 'costestimate$' AND p1.proname != 'update_pg_pwd_and_pg_group'; oid | proname ! ------+--------------- ! 2249 | pg_stat_reset ! (1 row) -- Look for conflicting proc definitions (same names and input datatypes). -- (This test should be dead code now that wehave the unique index ======================================================================
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> If you apply the pg_stat_reset() function patch you get this regression
> failure. Is this because it's returning a bool I guess? Shall I just fix
> the regression test to exclude this function?
No, you should fix the function definition. The sanity checks are there
for a reason.
regards, tom lane
Christopher Kings-Lynne wrote: > Hi guys, > > If you apply the pg_stat_reset() function patch you get this regression > failure. Is this because it's returning a bool I guess? Shall I just fix > the regression test to exclude this function? > AND p1.proname != 'update_pg_pwd_and_pg_group';> oid | proname> ! ------+---------------> ! 2249 | pg_stat_reset>! (1 row) Likely because this is now in CVS: DATA(insert OID = 2249 ( record PGNSP PGUID 4 t p t \054 0 0 oidin oidout i p f 0 -1 0 _null_ _null_ )); #define RECORDOID 2249 The Oids conflict. Joe
Hang on - I _can't_ fix the function defiition - it returns a bool and that's why it's failing. I can't have it returning a void because it's not possible. Check list of all other excluded functions as well... Chris ----- Original Message ----- From: "Tom Lane" <tgl@sss.pgh.pa.us> To: "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> Cc: "Hackers" <pgsql-hackers@postgresql.org> Sent: Friday, August 09, 2002 9:50 PM Subject: Re: [HACKERS] pg_stat_reset() weirdness > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > If you apply the pg_stat_reset() function patch you get this regression > > failure. Is this because it's returning a bool I guess? Shall I just fix > > the regression test to exclude this function? > > No, you should fix the function definition. The sanity checks are there > for a reason. > > regards, tom lane >
Ah doh - I thought it was catching it returning a boolean. I'll fix and resubmit. Chris ----- Original Message ----- From: "Joe Conway" <mail@joeconway.com> To: "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> Cc: "Hackers" <pgsql-hackers@postgresql.org> Sent: Friday, August 09, 2002 11:26 PM Subject: Re: [HACKERS] pg_stat_reset() weirdness > Christopher Kings-Lynne wrote: > > Hi guys, > > > > If you apply the pg_stat_reset() function patch you get this regression > > failure. Is this because it's returning a bool I guess? Shall I just fix > > the regression test to exclude this function? > > > > AND p1.proname != 'update_pg_pwd_and_pg_group'; > > oid | proname > > ! ------+--------------- > > ! 2249 | pg_stat_reset > > ! (1 row) > > Likely because this is now in CVS: > > DATA(insert OID = 2249 ( record PGNSP PGUID 4 t p t \054 0 0 > oidin oidout i p f 0 -1 0 _null_ _null_ )); > #define RECORDOID 2249 > > The Oids conflict. > > Joe >
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> Ah doh - I thought it was catching it returning a boolean. I'll fix and
> resubmit.
Unfortunately I don't believe Joe's theory --- an OID conflict between
pg_proc and pg_type shouldn't matter, and in any case the particular
sanity check that's failing is not looking at pg_type:
-- Look for illegal values in pg_proc fields.
-- NOTE: currently there are a few pg_proc entries that have prorettype = 0.
-- Someday that ought to be cleaned up.
SELECT p1.oid, p1.proname
FROM pg_proc as p1
WHERE (p1.prolang = 0 OR p1.prorettype = 0 OR p1.pronargs < 0 OR p1.pronargs > 16)AND p1.proname !~
'^pl[^_]+_call_handler$'ANDp1.proname !~ '^RI_FKey_'AND p1.proname !~ 'costestimate$'AND p1.proname !=
'update_pg_pwd_and_pg_group';
The pg_stat_reset definition I see in Chris' "round 3" patch does not
look like it should trigger this test. (I had misremembered the
previous discussion to think that he'd set prorettype = 0, but he
didn't.) So what's going wrong exactly? It needs investigation.
regards, tom lane
Tom Lane wrote: > Unfortunately I don't believe Joe's theory --- an OID conflict between > pg_proc and pg_type shouldn't matter, and in any case the particular > sanity check that's failing is not looking at pg_type: I guess I should know better than to jump to a conclusion. But I *was* under the impression we were supposed to use the unused_oids script to get a unique oid for a new function. > -- Look for illegal values in pg_proc fields. > -- NOTE: currently there are a few pg_proc entries that have prorettype = 0. > -- Someday that ought to be cleaned up. > SELECT p1.oid, p1.proname > FROM pg_proc as p1 > WHERE (p1.prolang = 0 OR p1.prorettype = 0 OR > p1.pronargs < 0 OR p1.pronargs > 16) > AND p1.proname !~ '^pl[^_]+_call_handler$' > AND p1.proname !~ '^RI_FKey_' > AND p1.proname !~ 'costestimate$' > AND p1.proname != 'update_pg_pwd_and_pg_group'; > > The pg_stat_reset definition I see in Chris' "round 3" patch does not > look like it should trigger this test. (I had misremembered the > previous discussion to think that he'd set prorettype = 0, but he > didn't.) So what's going wrong exactly? It needs investigation. > Actually, I don't see the regression failure here at all, now that I try the patch. Joe
Joe Conway <mail@joeconway.com> writes:
> I guess I should know better than to jump to a conclusion. But I *was*
> under the impression we were supposed to use the unused_oids script to
> get a unique oid for a new function.
Right, we do still insist that all hand-assigned OIDs be distinct, but
that is a matter of bookkeeping simplicity and possible debugging
advantage. The system should only care that the OIDs in any one catalog
are unique. (If it were to assume more, we'd have trouble after OID
wraparound, because we can't guarantee database-wide uniqueness then.
We *can* guarantee per-table uniqueness, by means of unique indexes
placed on OIDs --- you'll notice all the catalogs that use OIDs have
such indexes.)
> Actually, I don't see the regression failure here at all, now that I try
> the patch.
Hmm. Maybe Chris just needs a make clean/rebuild/etc?
regards, tom lane
> Joe Conway <mail@joeconway.com> writes: > > I guess I should know better than to jump to a conclusion. But I *was* > > under the impression we were supposed to use the unused_oids script to > > get a unique oid for a new function. unique_oids script???? Chris
Christopher Kings-Lynne wrote: > unique_oids script???? Look in src/include/catalog. You'll find duplicate_oids & unused_oids shell scripts. Joe
Can I have the updated version of this? --------------------------------------------------------------------------- Christopher Kings-Lynne wrote: > Ah doh - I thought it was catching it returning a boolean. I'll fix and > resubmit. > > Chris > > ----- Original Message ----- > From: "Joe Conway" <mail@joeconway.com> > To: "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> > Cc: "Hackers" <pgsql-hackers@postgresql.org> > Sent: Friday, August 09, 2002 11:26 PM > Subject: Re: [HACKERS] pg_stat_reset() weirdness > > > > Christopher Kings-Lynne wrote: > > > Hi guys, > > > > > > If you apply the pg_stat_reset() function patch you get this regression > > > failure. Is this because it's returning a bool I guess? Shall I just > fix > > > the regression test to exclude this function? > > > > > > > AND p1.proname != 'update_pg_pwd_and_pg_group'; > > > oid | proname > > > ! ------+--------------- > > > ! 2249 | pg_stat_reset > > > ! (1 row) > > > > Likely because this is now in CVS: > > > > DATA(insert OID = 2249 ( record PGNSP PGUID 4 t p t \054 0 0 > > oidin oidout i p f 0 -1 0 _null_ _null_ )); > > #define RECORDOID 2249 > > > > The Oids conflict. > > > > Joe > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073