Thread: Beta time

Beta time

From
Bruce Momjian
Date:
I want to mention that the number of patches submitted has dropped off
dramatically.  Seems people are prepared for beta and we should start
beta as soon as we can.  I think the current plan is Friday.

Also, I will be on vacation this week.  Tom will apply any patches that
look good.

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 


Re: Beta time

From
Thomas Lockhart
Date:
> I want to mention that the number of patches submitted has dropped off
> dramatically.  Seems people are prepared for beta and we should start
> beta as soon as we can.  I think the current plan is Friday.

I'm doing a substantial amount of work on the date/time types. Not
certain it will be ready for Friday. Will know more by then, of course
;)
                       - Thomas


Re: Beta time

From
"Christopher Kings-Lynne"
Date:
I spent an hour or two trying to get my ADD PRIMARY KEY patch to work but
I'm beginning to think my code is suffering from bit rot.  Basically, during
the iteration over the indices on the table, looking for other primary
indices, none are found.

I am checking the indexStruct->indisprimary field, but it always resolves to
false.  indisunique works fine.  It is a trivial change to the ADD UNIQUE
code, but it doesn't work.  Viewing the system catalogs and '\d' both show
the indices as primary, but the SearchSysCache funtion believes that they
are not.

Is DefineIndex for primary indices broken or something?

I have tried putting a CommandCounterIncrement() in there out of
desperation, but it does nothing.  Does anyone have any ideas?  Might have
to leave it for 7.3 I guess.

Chris

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org]On Behalf Of Bruce Momjian
> Sent: Tuesday, 18 September 2001 12:00 AM
> To: PostgreSQL-development
> Subject: [HACKERS] Beta time
>
>
> I want to mention that the number of patches submitted has dropped off
> dramatically.  Seems people are prepared for beta and we should start
> beta as soon as we can.  I think the current plan is Friday.
>
> Also, I will be on vacation this week.  Tom will apply any patches that
> look good.
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>



Re: Beta time

From
Tom Lane
Date:
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> I am checking the indexStruct->indisprimary field, but it always resolves to
> false.  indisunique works fine.  It is a trivial change to the ADD UNIQUE
> code, but it doesn't work.  Viewing the system catalogs and '\d' both show
> the indices as primary, but the SearchSysCache funtion believes that they
> are not.

Doesn't make any sense to me either.  Want to post your code?
        regards, tom lane


Anyone tried compiling postgresql with the Intel compilers?

From
Lincoln Yeoh
Date:
Hi has anyone tried Intel's compiler yet?

http://developer.intel.com/software/products/eval/

Just wondering what would happen.

Cheerio,
Link.



Re: Beta time

From
"Christopher Kings-Lynne"
Date:
Attached is the CONSTR_PRIMARY switch block from command.c.  I've marked the
problem test with '@@'.

Basically the patch all seems to work properly, except that it doesn't
recognise existing primarty keys.  ie. You can go ALTER TABLE test ADD
PRIMARY KEY(a) multiple times and it will keep adding a primary key.  My ADD
UNIQUE patch that has been committed is virtually identical, and has no such
problem.

Chris

> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> Sent: Tuesday, 18 September 2001 12:41 PM
> To: Christopher Kings-Lynne
> Cc: Bruce Momjian; PostgreSQL-development
> Subject: Re: [HACKERS] Beta time
>
>
> "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> > I am checking the indexStruct->indisprimary field, but it
> always resolves to
> > false.  indisunique works fine.  It is a trivial change to the
> ADD UNIQUE
> > code, but it doesn't work.  Viewing the system catalogs and
> '\d' both show
> > the indices as primary, but the SearchSysCache funtion believes
> that they
> > are not.
>
> Doesn't make any sense to me either.  Want to post your code?
>
>             regards, tom lane
>

Attachment

Re: Beta time

From
Tom Lane
Date:
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> Attached is the CONSTR_PRIMARY switch block from command.c.  I've marked the
> problem test with '@@'.

Hmmm .... this code has got a number of problems, but I don't see why
*that* would fail.  Anyone?

What I do see:

1. Should not "break" out of loop over indexes after detecting a
matching non-primary-key index.  This allows detection of the NOTICE
condition to distract you from detecting the ERROR condition on a
later index.  I'd suggest issuing the NOTICE inside the loop, actually,
and not breaking at all.  (See also #4)

2. What's with the "if (keyno > 0)"?  That breaks detection of
everything on indexes on system columns, eg OID.  (Of course, the
"rel_attrs[keyno - 1]" reference doesn't work for system columns,
but sticking your head in the sand is no answer.)

3. pfree'ing iname at the bottom doesn't strike me as a good
idea; isn't that possibly part of your input querytree?

4. If you're going to be so pedantic as to issue a warning notice about
a duplicate non-primary index, it'd be polite to give the name of that
index.  Else how shall I know which index you think I should drop?
        regards, tom lane


Re: Beta time

From
"Christopher Kings-Lynne"
Date:
> 1. Should not "break" out of loop over indexes after detecting a
> matching non-primary-key index.  This allows detection of the NOTICE
> condition to distract you from detecting the ERROR condition on a
> later index.  I'd suggest issuing the NOTICE inside the loop, actually,
> and not breaking at all.  (See also #4)

OK.

> 2. What's with the "if (keyno > 0)"?  That breaks detection of
> everything on indexes on system columns, eg OID.  (Of course, the
> "rel_attrs[keyno - 1]" reference doesn't work for system columns,
> but sticking your head in the sand is no answer.)

This is code that I've borrowed from elsewhere.  I'll have a good look at it
tho.

> 3. pfree'ing iname at the bottom doesn't strike me as a good
> idea; isn't that possibly part of your input querytree?

Hmmm.  OK.  What about in the case where iname is null and I give it a
makeObjectName?

> 4. If you're going to be so pedantic as to issue a warning notice about
> a duplicate non-primary index, it'd be polite to give the name of that
> index.  Else how shall I know which index you think I should drop?

I'll improve the messages.  As for me being pedantic - that's a result of
what you specified as the best behaviour should be when I posted on the
list!

You may also want to look at the CONSTR_UNIQUE block that's already been
committed, as it may also have similar issues.  Any fixes I make to PRIMARY,
I will also fix in UNIQUE...

Cheers,

Chris



Re: Beta time

From
Tom Lane
Date:
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
>> 3. pfree'ing iname at the bottom doesn't strike me as a good
>> idea; isn't that possibly part of your input querytree?

> Hmmm.  OK.  What about in the case where iname is null and I give it a
> makeObjectName?

Don't worry about it.  palloc'd space will be recovered anyway at end of
statement.  It's really not worth the code space to pfree every little
bit of space you might use, except in routines that could be executed
many times in a single query.
        regards, tom lane


Re: Beta time

From
Christopher Kings-Lynne
Date:
> 1. Should not "break" out of loop over indexes after detecting a
> matching non-primary-key index.  This allows detection of the NOTICE
> condition to distract you from detecting the ERROR condition on a
> later index.  I'd suggest issuing the NOTICE inside the loop, actually,
> and not breaking at all.  (See also #4)

I don't quite understand what you mean here?

> 2. What's with the "if (keyno > 0)"?  That breaks detection of
> everything on indexes on system columns, eg OID.  (Of course, the
> "rel_attrs[keyno - 1]" reference doesn't work for system columns,
> but sticking your head in the sand is no answer.)

That is that part of the code that I least understand, so I would
appreciate it if someone took 1 minute and told me how this _should_ be
written.  Note that I used this code from the ADD FOREIGN KEY stuff.
/* Look at key[i] in the index and check that it is over the same column   as key[i] in the constraint.  This is to
differentiatebetween (a,b)   and (b,a) */if (i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0){   int       keyno =
indexStruct->indkey[i];
   if (keyno > 0)   {      char  *name = NameStr(rel_attrs[keyno - 1]->attname);      if (strcmp(name, key->name) == 0)
keys_matched++;  }}
 

I admit I was confused as to why it's keyno - 1??

> 3. pfree'ing iname at the bottom doesn't strike me as a good
> idea; isn't that possibly part of your input querytree?

OK, gone.

> 4. If you're going to be so pedantic as to issue a warning notice about
> a duplicate non-primary index, it'd be polite to give the name of that
> index.  Else how shall I know which index you think I should drop?

I was going to do this, but then realised all I had access to in the
indexStruct was the oid of the index relation?  What's the easiest way of
retrieving the name of an index given it's oid, or the oid of it's
relation?

Once I've figured these probs out, I'll fix the ADD UNIQUE code as well.

Chris




Re: Beta time

From
Tom Lane
Date:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> I'd suggest issuing the NOTICE inside the loop, actually,
>> and not breaking at all.  (See also #4)

> I don't quite understand what you mean here?

Just do elog(NOTICE) inside the loop over indexes, rather than setting a
flag to do it later.  For that matter, I see no reason not to raise the
elog(ERROR) condition inside the loop, rather than setting a flag to
do it later.

> I admit I was confused as to why it's keyno - 1??

To make a zero-based C array index from the one-based attribute number.
But the problem with this code is it doesn't handle indexes on system
attributes such as OID, which have negative attribute numbers and are
not shown in rel->rd_att->attrs.  I'd be inclined to use get_attname,
and not bother with looking into the relcache rd_att structure at all.

> I was going to do this, but then realised all I had access to in the
> indexStruct was the oid of the index relation?  What's the easiest way of
> retrieving the name of an index given it's oid, or the oid of it's
> relation?

get_rel_name
        regards, tom lane