is the db class reentrant ? (following an old post of sigurd in 2005)

[Benoit Hamet]

Hi all,

I was trying to "quickly fix" the note apps for some notices about
get_var, when I notice some weird thing :

Notes display only one note ...

Further investigations let me point this :

in notes/inc/class.sonotes.inc.php I got :

around line 130 :
while ($this->db->next_record())
{
$ngrants = $this->grants[$this->db->f('note_owner')];
$notes[$this->db->f('note_id')] = array
(
'note_id' => $this->db->f('note_id'),
'owner_id' => $this->db->f('note_owner'),
'owner' =>
$GLOBALS['phpgw']->accounts->id2name($this->db->f('note_owner')),
'access' => $this->db->f('note_access'),
'date' =>
$GLOBALS['phpgw']->common->show_date($this->db->f('note_date')),
'cat_id' => $this->db->f('note_category'),
'content' => $this->db->f('note_content', true),
'grants' => $ngrants
);
}

of course, removing the line 'owner' =>
$GLOBALS['phpgw']->accounts->id2name($this->db->f('note_owner')),

solve the problem, which let me think about a reentrant problem (so a
new query is done using the same db object, and so next_record() is
totaly wrong next time ...).

I Guess we already discuss this [Yeah : search for "Is there some
problem using adodb?" in 2005 archives] ... but do we have an "elegant"
solution for this kind of problem ?
clone the db object in the sonotes ? (beurk)
perhaps better, let accounts having it's own cloned db object (little
better, since like that it won't hurt when using ldap as backend).
avoid this kind of code ? :)

Regards,

Caeies.

[Chris Weiss]

Post by Benoit Hamet
Hi all,
I was trying to "quickly fix" the note apps for some notices about
Notes display only one note ...
while ($this->db->next_record())
{
$ngrants = $this->grants[$this->db->f('note_owner')];
$notes[$this->db->f('note_id')] = array
(
'note_id' => $this->db->f('note_id'),
'owner_id' => $this->db->f('note_owner'),
'owner' =>
$GLOBALS['phpgw']->accounts->id2name($this->db->f('note_owner')),
'access' => $this->db->f('note_access'),
'date' =>
$GLOBALS['phpgw']->common->show_date($this->db->f('note_date')),
'cat_id' => $this->db->f('note_category'),
'content' => $this->db->f('note_content', true),
'grants' => $ngrants
);
}
of course, removing the line 'owner' =>
$GLOBALS['phpgw']->accounts->id2name($this->db->f('note_owner')),
solve the problem, which let me think about a reentrant problem (so a
new query is done using the same db object, and so next_record() is
totaly wrong next time ...).
I Guess we already discuss this [Yeah : search for "Is there some
problem using adodb?" in 2005 archives] ... but do we have an "elegant"
solution for this kind of problem ?
clone the db object in the sonotes ? (beurk)
perhaps better, let accounts having it's own cloned db object (little
better, since like that it won't hurt when using ldap as backend).
avoid this kind of code ? :)

I'd go for the latter. if we could always lookup the name int eh db
i'd go with adding a JOIN to the first query, but if that won't work
with ldap, then having the accounts class self contained is the better
option.

however, with adodb we can make it reentrant, just that the old db
class that wraps it wasn't designed that way.

[Dave Hall]

Post by Chris Weiss

Post by Benoit Hamet
solve the problem, which let me think about a reentrant problem (so a
new query is done using the same db object, and so next_record() is
totaly wrong next time ...).
I Guess we already discuss this [Yeah : search for "Is there some
problem using adodb?" in 2005 archives] ... but do we have an "elegant"
solution for this kind of problem ?
clone the db object in the sonotes ? (beurk)
perhaps better, let accounts having it's own cloned db object (little
better, since like that it won't hurt when using ldap as backend).
avoid this kind of code ? :)

I'd go for the latter. if we could always lookup the name int eh db
i'd go with adding a JOIN to the first query, but if that won't work
with ldap, then having the accounts class self contained is the better
option.
however, with adodb we can make it reentrant, just that the old db
class that wraps it wasn't designed that way.

I think it is best to make the db object a singleton and use PDO, but I
don't think that will happen in this release. I think we are stuck with
the same circa 1999 design from PHPLib for this release.

Cloning the db object is not sustainable. I have seen cases where we
have had more than 10 db objects created for rendering 1 simple list of
records. This chews up a lot of resources and memory on the server.

Cheers

Dave

[Chris Weiss]

Post by Chris Weiss

Post by Benoit Hamet
solve the problem, which let me think about a reentrant problem (so a
new query is done using the same db object, and so next_record() is
totaly wrong next time ...).
I Guess we already discuss this [Yeah : search for "Is there some
problem using adodb?" in 2005 archives] ... but do we have an "elegant"
solution for this kind of problem ?
clone the db object in the sonotes ? (beurk)
perhaps better, let accounts having it's own cloned db object (little
better, since like that it won't hurt when using ldap as backend).
avoid this kind of code ? :)

I'd go for the latter. if we could always lookup the name int eh db
i'd go with adding a JOIN to the first query, but if that won't work
with ldap, then having the accounts class self contained is the better
option.
however, with adodb we can make it reentrant, just that the old db
class that wraps it wasn't designed that way.

I think it is best to make the db object a singleton and use PDO, but I
don't think that will happen in this release. I think we are stuck with
the same circa 1999 design from PHPLib for this release.
Cloning the db object is not sustainable. I have seen cases where we
have had more than 10 db objects created for rendering 1 simple list of
records. This chews up a lot of resources and memory on the server.
Cheers
Dave

right, I was speaking "for the short term". fixing it the right way
would be a rather major change, and we don't don't need any of that
right now.

[Dave Hall]

Post by Benoit Hamet
Hi all,
I was trying to "quickly fix" the note apps for some notices about
Notes display only one note ...

Nothing weird, the db class only allows one result set at a time. See
below for the fix.

Post by Benoit Hamet
while ($this->db->next_record())
{
$ngrants = $this->grants[$this->db->f('note_owner')];
$notes[$this->db->f('note_id')] = array
(
'note_id' => $this->db->f('note_id'),
'owner_id' => $this->db->f('note_owner'),
'owner' =>
$GLOBALS['phpgw']->accounts->id2name($this->db->f('note_owner')),
'access' => $this->db->f('note_access'),
'date' =>
$GLOBALS['phpgw']->common->show_date($this->db->f('note_date')),
'cat_id' => $this->db->f('note_category'),
'content' => $this->db->f('note_content', true),
'grants' => $ngrants
);
}
of course, removing the line 'owner' =>
$GLOBALS['phpgw']->accounts->id2name($this->db->f('note_owner')),
solve the problem, which let me think about a reentrant problem (so a
new query is done using the same db object, and so next_record() is
totaly wrong next time ...).

IMHO this should be moved the bo layer. This is business logic not
storage logic. Fetch the data quickly then move on. In the bo layer
you can use a foreach loop which passes the element by reference.

Something like this

// example using new trunk code
foreach ( $notes as &$note )
{
$note['owner'] = (string) $GLOBALS['phpgw']->accounts->get($note['owner_id'];
}

Gotta love PHP 5 :)

Cheers

Dave