Thread: PyGreSQL, suggestion for DB wrapper class
Here is another suggested patch for PyGreSQL: There is an ugly little problem with the DB wrapper class. In pg.py the attributes of DB are defined as being the same as the attributes of the corresponding pgobject "db", using the following -------- piece of Python code in pg.py ---------- # Create convience methods ... for e in ( 'query', ... , 'error', 'status', ...): if not hasattr(self,e) and hasattr(self.db,e): exec 'self.%s = self.db.%s' % ( e, e ) ---------------------------------------------------- The problem is that the attributes of db (which are read only) are not static (they are actually function calls to PostgreSQL), especially "status" and "error", but those attributes are copied and this is done only once when initializing the DB object. So, in effect, only the attribute "db.error" of a DB instance will be updated, but not the attribute "error". Same with "status". The following Python session demonstrates the problem: >>> from pg import DB >>> db=DB('template1') >>> db.query("this is an invalid query") _pg.error: ERROR: parser: parse error at or near "this" >>> db.error '' >>> db.db.error 'ERROR: parser: parse error at or near "this"\n' Suggested solution: Don't copy the (read only) attributes of the pgobject to the DB object, but only the methods, and all of them, like this: --------------- change in pg.py ------------------ # Create convience methods, in a way that is still overridable. for e in self.db.__methods__: setattr(self, e, getattr(self.db, e)) ---------------------------------------------------- Furthermore, make an addition to the documentation of the DB wrapper class (i.e. in pygresql-pg-db.html): After the sentence "All pgobject methods are included in this class also." add the following sentence "The pgobject read-only attributes can be accessed py adding the prefix 'db.' to them." Christoph Zwerschke Zentrale Univerwaltung Heidelberg
Would you send in a patch on this? It would be helpful. Thanks. --------------------------------------------------------------------------- Christoph Zwerschke wrote: > Here is another suggested patch for PyGreSQL: > > > There is an ugly little problem with the DB wrapper class. > > In pg.py the attributes of DB are defined as being the same as > the attributes of the corresponding pgobject "db", using the following > > -------- piece of Python code in pg.py ---------- > # Create convience methods ... > for e in ( 'query', ... , 'error', 'status', ...): > if not hasattr(self,e) and hasattr(self.db,e): > exec 'self.%s = self.db.%s' % ( e, e ) > ---------------------------------------------------- > > The problem is that the attributes of db (which are read only) > are not static (they are actually function calls to PostgreSQL), > especially "status" and "error", but those attributes are copied > and this is done only once when initializing the DB object. > > So, in effect, only the attribute "db.error" of a DB instance > will be updated, but not the attribute "error". Same with "status". > > The following Python session demonstrates the problem: > > >>> from pg import DB > >>> db=DB('template1') > >>> db.query("this is an invalid query") > _pg.error: ERROR: parser: parse error at or near "this" > >>> db.error > '' > >>> db.db.error > 'ERROR: parser: parse error at or near "this"\n' > > > Suggested solution: > > Don't copy the (read only) attributes of the pgobject to the > DB object, but only the methods, and all of them, like this: > > --------------- change in pg.py ------------------ > # Create convience methods, in a way that is still overridable. > for e in self.db.__methods__: > setattr(self, e, getattr(self.db, e)) > ---------------------------------------------------- > > Furthermore, make an addition to the documentation of the > DB wrapper class (i.e. in pygresql-pg-db.html): > After the sentence "All pgobject methods are included in this class also." > add the following sentence "The pgobject read-only attributes can be > accessed py adding the prefix 'db.' to them." > > > Christoph Zwerschke > Zentrale Univerwaltung Heidelberg > > > ---------------------------(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, Pennsylvania 19073
> Would you send in a patch on this? It would be helpful. Thanks. Here are the two patches (one for the python module and one for the doco). I hope the format is ok. Gtx Christoph Zwerschke
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --------------------------------------------------------------------------- Christoph Zwerschke wrote: > > Would you send in a patch on this? It would be helpful. Thanks. > > Here are the two patches (one for the python module and one for the doco). I > hope the format is ok. > > Gtx > Christoph Zwerschke [ Attachment, skipping... ] [ Attachment, skipping... ] -- 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, Pennsylvania 19073
Patch applied. Thanks. I normally need context diffs (diff -c), but your diff had enough context. --------------------------------------------------------------------------- Christoph Zwerschke wrote: > > Would you send in a patch on this? It would be helpful. Thanks. > > Here are the two patches (one for the python module and one for the doco). I > hope the format is ok. > > Gtx > Christoph Zwerschke [ Attachment, skipping... ] [ Attachment, skipping... ] -- 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, Pennsylvania 19073