From: Ray Z. <rz...@co...> - 2002-12-23 21:10:03
|
Chris, It seems like there are a few things broken in SPOPS-0.72 which are causing a bunch of my ESPOPS tests to fail. Two things I noticed are that the 'if_match' handling of field_update() seems to be broken and all_field_types() returns a hash with all undefs for the values. I'm pretty sure changing ... $rv = ( $rv != 0 ); ... to ... $rv = ( $rv ne '0' ); ... near the end of field_update() is a bug. Normally, if the update does not modify any rows it will return a "true" zero value, namely '0E0'. I suspect my other problems are related to the changes to the way type_info is handled. Merry Christmas! -- Ray Zimmerman / e-mail: rz...@co... / 428-B Phillips Hall Sr Research / phone: (607) 255-9645 / Cornell University Associate / FAX: (815) 377-3932 / Ithaca, NY 14853 |
From: Chris W. <ch...@cw...> - 2003-01-03 04:28:03
|
Ray Zimmerman wrote: > I'm back after some time off ... Hope your holidays were excellent. > First, I'm getting test 37 failing for t/30_dbi.t ... > > not ok 37 - Field update (multiple object) execution > # Failed test (t/30_dbi.t at line 260) > # got: '2' > # expected: '3' > > ... and a bunch of my ESPOPS tests (which work fine with SPOPS-0.70) > still break. Let me respond to your previous e-mail on field_update() ... Hm, that's interesting. I probably should have tried this out on another database before releasing... > As it stands, I think field_update() always returns true, even if no > rows were affected. > > So, I still think the "$rv = ( $rv != 0 )" code from 0.70 is what we (or > at least I :-) want. I just ran the tests with SQLite and got errors in that DBI test (plus one other) as well. So this will be going back. Oh well, confidence borne of ignornace and all that. > Ah ... whoops ... all_field_types() was an ESPOPS method that calls > db_discover_types(), as you suggest, with the various classes/tables > that make up an object with inherited fields. > > However, it looks like db_discover_types() is now returning a > SPOPS::DBI::TypeInfo object instead of a simple hash ... I never > installed SPOPS-0.71 so I missed this change ... lemme go have a look at > the docs for this class ... > > ... hey, the docs thank me for pointing out the need for the > functionality in this module ... am I forgetting something or are you > just starting to put my name in the credits out of habit :-) Ha! I should start pasting links to the email archives.... > So your example of how to use db_discover_types() is no longer valid, is > it? Nope. I must have pulled that from the wayback machine by accident. Your example is correct and has been placed into the docs for SPOPS::SQLInterface. Thanks. Chris -- Chris Winters (ch...@cw...) Building enterprise-capable snack solutions since 1988. |
From: Ray Z. <rz...@co...> - 2003-01-03 12:27:23
|
At 11:41 PM -0500 1/2/03, Chris Winters wrote: >>So your example of how to use db_discover_types() is no longer valid, is it? > >Nope. I must have pulled that from the wayback machine by accident. >Your example is correct and has been placed into the docs for >SPOPS::SQLInterface. Thanks. Well, it wasn't exactly *my* example ... after all I did pretty much get it straight from the SPOPS::DBI::TypeInfo docs. -- Ray Zimmerman / e-mail: rz...@co... / 428-B Phillips Hall Sr Research / phone: (607) 255-9645 / Cornell University Associate / FAX: (815) 377-3932 / Ithaca, NY 14853 |
From: Chris W. <ch...@cw...> - 2003-01-03 13:34:20
|
Ray Zimmerman wrote: > Ah ... case-sensitivity stuff ... now I remember vaguely why you're > doing this ... I'm too young for my memory to be going already :-) Tell me about it. Sometimes I wish I were still doing Perl full-time... Can you grab/test SPOPS 0.74 before I release it and run the ESPOPS tests against it to see how they go? http://www.cwinters.com/raw/SPOPS-0.74.tar.gz Thanks, Chris -- Chris Winters (ch...@cw...) Building enterprise-capable snack solutions since 1988. |
From: Ray Z. <rz...@co...> - 2003-01-03 14:00:30
|
At 8:48 AM -0500 1/3/03, Chris Winters wrote: >Tell me about it. Sometimes I wish I were still doing Perl full-time... Yup. The better I get with Perl the more I like it. >Can you grab/test SPOPS 0.74 before I release it and run the ESPOPS >tests against it to see how they go? > > http://www.cwinters.com/raw/SPOPS-0.74.tar.gz After adding a call to as_hash() at the appropriate place in ESPOPS, all of the SPOPS and ESPOPS tests are passing nicely for me again. Ship it! -- Ray Zimmerman / e-mail: rz...@co... / 428-B Phillips Hall Sr Research / phone: (607) 255-9645 / Cornell University Associate / FAX: (815) 377-3932 / Ithaca, NY 14853 |
From: Chris W. <ch...@cw...> - 2002-12-23 22:14:00
|
Ray Zimmerman wrote: > It seems like there are a few things broken in SPOPS-0.72 which are > causing a bunch of my ESPOPS tests to fail. > > Two things I noticed are that the 'if_match' handling of field_update() > seems to be broken and all_field_types() returns a hash with all undefs > for the values. > > I'm pretty sure changing ... > > $rv = ( $rv != 0 ); > > ... to ... > > $rv = ( $rv ne '0' ); > > ... near the end of field_update() is a bug. Normally, if the update > does not modify any rows it will return a "true" zero value, namely '0E0'. I think you're right. I modified this in CVS to just return $rv instead. I'll probably put out a new version shortly, just as soon as I get something built to do automatic document building for the SPOPS website. > I suspect my other problems are related to the changes to the way > type_info is handled. How were you retrieving/using the field types? > Merry Christmas! Same to you! Chris -- Chris Winters (ch...@cw...) Building enterprise-capable snack solutions since 1988. |
From: Ray Z. <rz...@co...> - 2002-12-23 22:39:26
|
At 5:52 PM -0500 12/23/02, Chris Winters wrote: >I think you're right. I modified this in CVS to just return $rv >instead. I'll probably put out a new version shortly, just as soon >as I get something built to do automatic document building for the >SPOPS website. I think that's still wrong ... $rv = ( $rv != 0 ) was correct. >>I suspect my other problems are related to the changes to the way >>type_info is handled. > >How were you retrieving/using the field types? I was calling $self->all_field_types() then checking types against constants defined in DBI. -- Ray Zimmerman / e-mail: rz...@co... / 428-B Phillips Hall Sr Research / phone: (607) 255-9645 / Cornell University Associate / FAX: (815) 377-3932 / Ithaca, NY 14853 |
From: Chris W. <ch...@cw...> - 2002-12-24 15:01:36
|
Ray Zimmerman wrote: > I think that's still wrong ... $rv = ( $rv != 0 ) was correct. But this removes information that might be useful: returning $rv gives an indication of the number of rows affected while still retaining the "return false if no rows affected" semantics. So you can still do: my $rows = $class->field_update( ... ); if ( $rows ) { print "$rows rows affected"; } else { print "No rows affected"; } >> How were you retrieving/using the field types? > > I was calling $self->all_field_types() then checking types against > constants defined in DBI. You should be able to use something like: my $type_hash = $class->db_discover_types( $class->table_name ); foreach my $field ( keys %{ $type_hash } ) { print "$field is DBI type $type_hash->{ $field }\n"; } SQLInterface->db_discover_types is still the front end for retriving the types, and under the covers we're still using DBI types, just getting them in a cleaner manner. Happy holidays! Chris -- Chris Winters (ch...@cw...) Building enterprise-capable snack solutions since 1988. |
From: Ray Z. <rz...@co...> - 2003-01-02 19:56:22
|
I'm back after some time off ... First, I'm getting test 37 failing for t/30_dbi.t ... not ok 37 - Field update (multiple object) execution # Failed test (t/30_dbi.t at line 260) # got: '2' # expected: '3' ... and a bunch of my ESPOPS tests (which work fine with SPOPS-0.70) still break. Let me respond to your previous e-mail on field_update() ... At 10:40 AM -0500 12/24/02, Chris Winters wrote: >Ray Zimmerman wrote: >>I think that's still wrong ... $rv = ( $rv != 0 ) was correct. > >But this removes information that might be useful: returning $rv >gives an indication of the number of rows affected while still >retaining the "return false if no rows affected" semantics. So you >can still do: > > my $rows = $class->field_update( ... ); > if ( $rows ) { > print "$rows rows affected"; > } > else { > print "No rows affected"; > } If I understand things correctly, this code will not do what you expect. The problem is precisely that it doesn't retain the "return false if no rows affected" semantics. If no rows are affected $rows will be '0E0' which is a true value for which $rows == 0 is also true. From the DBI docs ... "A successful execute always returns true regardless of the number of rows affected, even if it's zero" ... and ... "For a non-SELECT statement, execute returns the number of rows affected, if known. If no rows were affected, then execute returns ``0E0'', which Perl will treat as 0 but will regard as true. Note that it is not an error for no rows to be affected by a statement. If the number of rows affected is not known, then execute returns -1." As it stands, I think field_update() always returns true, even if no rows were affected. So, I still think the "$rv = ( $rv != 0 )" code from 0.70 is what we (or at least I :-) want. >>>How were you retrieving/using the field types? >> >>I was calling $self->all_field_types() then checking types against >>constants defined in DBI. > >You should be able to use something like: > > my $type_hash = $class->db_discover_types( $class->table_name ); > foreach my $field ( keys %{ $type_hash } ) { > print "$field is DBI type $type_hash->{ $field }\n"; > } > >SQLInterface->db_discover_types is still the front end for retriving >the types, and under the covers we're still using DBI types, just >getting them in a cleaner manner. Ah ... whoops ... all_field_types() was an ESPOPS method that calls db_discover_types(), as you suggest, with the various classes/tables that make up an object with inherited fields. However, it looks like db_discover_types() is now returning a SPOPS::DBI::TypeInfo object instead of a simple hash ... I never installed SPOPS-0.71 so I missed this change ... lemme go have a look at the docs for this class ... ... hey, the docs thank me for pointing out the need for the functionality in this module ... am I forgetting something or are you just starting to put my name in the credits out of habit :-) So your example of how to use db_discover_types() is no longer valid, is it? Shouldn't it be ... my $type_info = $class->db_discover_types( $class->table_name ); foreach my $field ( $type_info->get_fields ) { print "$field is DBI type " . $type_info->get_type( $field ) . "\n"; } ... maybe I missed it, but I didn't see a way to get back the old hash that my code was using. In any case, I can change my code to use the new TypeInfo object instead of the hash ... it's not a big problem. -- Ray Zimmerman / e-mail: rz...@co... / 428-B Phillips Hall Sr Research / phone: (607) 255-9645 / Cornell University Associate / FAX: (815) 377-3932 / Ithaca, NY 14853 |
From: Chris W. <ch...@cw...> - 2003-01-03 05:11:10
|
Ray Zimmerman wrote: > ... maybe I missed it, but I didn't see a way to get back the old hash > that my code was using. In any case, I can change my code to use the new > TypeInfo object instead of the hash ... it's not a big problem. Just a follow-up: I added an 'as_hash()' method to the SPOPS::DBI::TypeInfo class, so you can do: my %type_map = $type_info->as_hash; foreach my $field ( keys %type_map ) { print "Field $field is type $type_map{ $field }\n"; } You may still run into the case-sensitivity problem from before (fields are in whatever case specified or as they were retrieved from the database), but that's your problem now that there's an alternative ;-) Chris -- Chris Winters (ch...@cw...) Building enterprise-capable snack solutions since 1988. |
From: Ray Z. <rz...@co...> - 2003-01-03 12:28:41
|
At 12:24 AM -0500 1/3/03, Chris Winters wrote: >Just a follow-up: I added an 'as_hash()' method to the >SPOPS::DBI::TypeInfo class, so you can do: > > my %type_map = $type_info->as_hash; > foreach my $field ( keys %type_map ) { > print "Field $field is type $type_map{ $field }\n"; > } > >You may still run into the case-sensitivity problem from before >(fields are in whatever case specified or as they were retrieved >from the database), but that's your problem now that there's an >alternative ;-) Ah ... case-sensitivity stuff ... now I remember vaguely why you're doing this ... I'm too young for my memory to be going already :-) -- Ray Zimmerman / e-mail: rz...@co... / 428-B Phillips Hall Sr Research / phone: (607) 255-9645 / Cornell University Associate / FAX: (815) 377-3932 / Ithaca, NY 14853 |