Re: python patch

From: Greg Copeland <greg(at)CopelandConsulting(dot)Net>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Christopher Kings-Lynne <chriskl(at)familyhealth(dot)com(dot)au>, Rod Taylor <rbt(at)zort(dot)ca>, PostgresSQL Hackers Mailing List <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: python patch
Date: 2002-08-15 03:39:44
Message-ID: 1029382784.13681.222.camel@mouse.copelandconsulting.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks.

-Greg

On Wed, 2002-08-14 at 22:34, Bruce Momjian wrote:
>
> OK, I have applied all three of Greg's python patches.
>
> ---------------------------------------------------------------------------
>
> Christopher Kings-Lynne wrote:
> > Yep - alright, just commit it I guess.
> >
> > Chris
> >
> > > -----Original Message-----
> > > From: Greg Copeland [mailto:greg(at)copelandconsulting(dot)net]
> > > Sent: Thursday, 15 August 2002 11:09 AM
> > > To: Rod Taylor
> > > Cc: Christopher Kings-Lynne; Bruce Momjian; PostgresSQL Hackers Mailing
> > > List
> > > Subject: Re: [HACKERS] python patch
> > >
> > >
> > > Well, I tend to agree with that. Overall, I can't say that I see bad
> > > things coming out of accepting the patch as is. It's not exactly
> > > causing an extra join or other wise a significant waste of resources.
> > > At worst, it appears to be ambiguous. Since Christopher has not offered
> > > any additional follow up, can we assume that he agrees? In not, please
> > > let me know and I'll resubmit patch #2.
> > >
> > > In the mean time, patches #1 and #3 should be good to go. Bruce, feel
> > > free to apply those whenever time allows.
> > >
> > > Thanks,
> > > Greg Copeland
> > >
> > >
> > > On Mon, 2002-08-12 at 18:33, Rod Taylor wrote:
> > > > All of that said, the cost of the check is so small it may save someones
> > > > ass some day when they have a corrupted catalog and the below
> > > > assumptions are no longer true.
> > > >
> > > > On Mon, 2002-08-12 at 18:40, Greg Copeland wrote:
> > > > > On Sun, 2002-08-11 at 21:15, Christopher Kings-Lynne wrote:
> > > > > > > Not a problem. I would rather them be correct.
> > > > > > >
> > > > > > > Worth noting that the first patch is what attempts to fix
> > > the long ->
> > > > > > > int overflow issue. The second patch attempts to resolve
> > > "attisdropped"
> > > > > > > column use issues with the python scripts. The third
> > > patch addresses
> > > > > > > issues generated by the implicate to explicate use of "cascade".
> > > > > > >
> > > > > > > I assume your reservations are only with the second patch
> > > and not the
> > > > > > > first and third patches?
> > > > > >
> > > > > > Correct. I'm pretty sure you don't need to exclude
> > > attisdropped from the
> > > > > > primary key list because all it's doing is finding the
> > > column that a primary
> > > > > > key is over and that should never be over a dropped column. I can't
> > > > > > remember what you said the second query did?
> > > > >
> > > > >
> > > > > Hmmm. Sounds okay but I'm just not sure that holds true (as I
> > > > > previously stated, I'm ignorant on the topic). Obviously
> > > I'll defer to
> > > > > you on this.
> > > > >
> > > > > Here's the queries and what they do:
> > > > >
> > > > >
> > > > > >From pg.py:
> > > > > Used to locate primary keys -- or so the comment says. It
> > > does create a
> > > > > dictionary of keys and attribute values for each returned row so I
> > > > > assume it really is attempting to do something of the like.
> > > > >
> > > > > SELECT pg_class.relname, pg_attribute.attname
> > > > > FROM pg_class, pg_attribute, pg_index
> > > > > WHERE pg_class.oid = pg_attribute.attrelid AND
> > > > > pg_class.oid = pg_index.indrelid AND
> > > > > pg_index.indkey[0] = pg_attribute.attnum AND
> > > > > pg_index.indisprimary = 't' AND
> > > > > pg_attribute.attisdropped = 'f' ;
> > > > >
> > > > > So, everyone is in agreement that any attribute which is indexed as a
> > > > > primary key will never be able to have attisdtopped = 't'?
> > > > >
> > > > > According to the code:
> > > > > SELECT pg_attribute.attname, pg_type.typname
> > > > > FROM pg_class, pg_attribute, pg_type
> > > > > WHERE pg_class.relname = '%s' AND
> > > > > pg_attribute.attnum > 0 AND
> > > > > pg_attribute.attrelid = pg_class.oid AND
> > > > > pg_attribute.atttypid = pg_type.oid AND
> > > > > pg_attribute.attisdropped = 'f' ;
> > > > >
> > > > > is used to obtain all attributes (column names) and their types for a
> > > > > given table ('%s'). It then attempts to build a column/type
> > > cache. I'm
> > > > > assuming that this really does need to be there. Please correct
> > > > > accordingly.
> > > > >
> > > > >
> > > > > >From syscat.py:
> > > > > SELECT bc.relname AS class_name,
> > > > > ic.relname AS index_name, a.attname
> > > > > FROM pg_class bc, pg_class ic, pg_index i, pg_attribute a
> > > > > WHERE i.indrelid = bc.oid AND i.indexrelid = bc.oid
> > > > > AND i.indkey[0] = a.attnum AND a.attrelid = bc.oid
> > > > > AND i.indproc = '0'::oid AND a.attisdropped = 'f'
> > > > > ORDER BY class_name, index_name, attname ;
> > > > >
> > > > > According to the nearby documentation, it's supposed to be fetching a
> > > > > list of "all simple indicies". If that's the case, is it
> > > safe to assume
> > > > > that any indexed column will never have attisdropped = 't'? If so, we
> > > > > can remove that check from the file as well. Worth pointing out, this
> > > > > is from syscat.py, which is sample source and not used as actual
> > > > > interface. So, worse case, it would appear to be redundant in nature
> > > > > with no harm done.
> > > > >
> > > > > This should conclude the patched items offered in the second patch.
> > > > >
> > > > > What ya think?
> > > > >
> > > > > Thanks,
> > > > > Greg
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > ---------------------------(end of broadcast)---------------------------
> > > > TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
> > >
> > >
> >
> >
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Curt Sampson 2002-08-15 03:41:26 Re: [COMMITTERS] pgsql-server/src backend/tcop/postgres.cbacke
Previous Message Bruce Momjian 2002-08-15 03:34:25 Re: python patch