Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / lazarus / 2012-07

Propose: dont try to open TDBLookup* ListSource automatically

10 messages · 6 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Luiz Americo Pereira Camara

Currently the db lookup controls tries to open the dataset linked to ListSource if is closed at lookup initialization.

The initialization occurs after the control is loaded (streamed) or when a related db property is changed in the control. This behavior can leads to opening the dataset at unpredictable time or even twice (if the programmer opens the dataset after loading of the control). Also it can trigger events like AfterOpen in improper moments, e.g., data structures not initialized.

Removing this feature, that is LCL specific, can have side effects in projects that do not open explicitly the ListSource dataset or closes it when the lookup control is still in use.

Recently two bugs were reported due to this behavior (see http://bugs.freepascal.org/view.php?id=22387
http://bugs.freepascal.org/view.php?id=22337
http://lazarus.freepascal.org/index.php/topic,17355.msg96112.html )

So, removing this feature has the benefits of a cleaner code and better user control when to open the dataset thus avoiding potential bugs and decreased performance. The drawback is that it can affect programs that relies in that behavior although it should be easily fixable and if there's a good moment for such changes is now since 1.0 has already branched.

Luiz

Reply by email

Michael Van Canneyt
to Luiz Americo Pereira Camara
Luiz Americo Pereira Camara wrote 24 lines
Currently the db lookup controls tries to open the dataset linked to ListSource if is closed at lookup initialization. The initialization occurs after the control is loaded (streamed) or when a related db property is changed in the control. This behavior can leads to opening the dataset at unpredictable time or even twice (if the programmer opens the dataset after loading of the control). Also it can trigger events like AfterOpen in improper moments, e.g., data structures not initialized. Removing this feature, that is LCL specific, can have side effects in projects that do not open explicitly the ListSource dataset or closes it when the lookup control is still in use. Recently two bugs were reported due to this behavior (see http://bugs.freepascal.org/view.php?id=22387 http://bugs.freepascal.org/view.php?id=22337 http://lazarus.freepascal.org/index.php/topic,17355.msg96112.html ) So, removing this feature has the benefits of a cleaner code and better user control when to open the dataset thus avoiding potential bugs and decreased performance. The drawback is that it can affect programs that relies in that behavior although it should be easily fixable and if there's a good moment for such changes is now since 1.0 has already branched.

Why not introduce a boolean property OpenLookupDataset to control this behaviour ?

Michael.

Reply by email

Ludo Brands
to Luiz Americo Pereira Camara: Currently the db lookup controls tries to open the dataset linked to ListSource if is…
Currently the db lookup controls tries to open the dataset linked to ListSource if is closed at lookup initialization.

You probably guessed it but I'm voting for removing this feature. Opening of datasets and establishing database connections behind the back of the programmer is always a bad idea.

The try open when loaded (streamed), even in the fixed version, has also the nasty side effect of not being deterministic. A programmer can't easily influence the loading order of components and a change in the order will change the behavior of the program. The fix in 22337 solves the exception but not the unpredictable behavior. If the link dataset and connection are loaded before the dblookup it will open the dataset, else it won't. So your program can start up with (some) dblookups pre-loaded or not.

We shouldn't support users counting on unpredictable behavior.

Ludo

Reply by email

Michael Van Canneyt
to Ludo Brands
Ludo Brands wrote 7 lines
> Why not introduce a boolean property OpenLookupDataset to > control this behaviour ? > Default false, I hope. Somebody that wants shaky behavior should do so willingly and "sign" for it.

If that makes you happy, why not ? :)

Michael.

Reply by email

Reinier Olislagers
to Ludo Brands: You probably guessed it but I'm voting for removing this feature. Opening of datasets and…
Ludo Brands wrote 19 lines
> Currently the db lookup controls tries to open the dataset linked to > ListSource if is closed at lookup initialization. > You probably guessed it but I'm voting for removing this feature. Opening of datasets and establishing database connections behind the back of the programmer is always a bad idea. The try open when loaded (streamed), even in the fixed version, has also the nasty side effect of not being deterministic. A programmer can't easily influence the loading order of components and a change in the order will change the behavior of the program. The fix in 22337 solves the exception but not the unpredictable behavior. If the link dataset and connection are loaded before the dblookup it will open the dataset, else it won't. So your program can start up with (some) dblookups pre-loaded or not. We shouldn't support users counting on unpredictable behavior.

Totally agreed.

Reply by email

Marcos Douglas
to Reinier Olislagers: Totally agreed.
Reinier Olislagers wrote 21 lines
On 9-7-2012 10:00, Ludo Brands wrote: > >> Currently the db lookup controls tries to open the dataset linked to >> ListSource if is closed at lookup initialization. >> > > You probably guessed it but I'm voting for removing this feature. Opening of > datasets and establishing database connections behind the back of the > programmer is always a bad idea. > > The try open when loaded (streamed), even in the fixed version, has also the > nasty side effect of not being deterministic. A programmer can't easily > influence the loading order of components and a change in the order will > change the behavior of the program. The fix in 22337 solves the exception > but not the unpredictable behavior. If the link dataset and connection are > loaded before the dblookup it will open the dataset, else it won't. So your > program can start up with (some) dblookups pre-loaded or not. > > We shouldn't support users counting on unpredictable behavior. > Totally agreed.

+1

Marcos Douglas

Reply by email

Antonio Fortuny
to Marcos Douglas

Le 09/07/2012 14:02, Marcos Douglas a ?crit :

Show quoted text 24 lines
On Mon, Jul 9, 2012 at 5:27 AM, Reinier Olislagers wrote: > On 9-7-2012 10:00, Ludo Brands wrote: >>> Currently the db lookup controls tries to open the dataset linked to >>> ListSource if is closed at lookup initialization. >>> >> You probably guessed it but I'm voting for removing this feature. Opening of >> datasets and establishing database connections behind the back of the >> programmer is always a bad idea. >> >> The try open when loaded (streamed), even in the fixed version, has also the >> nasty side effect of not being deterministic. A programmer can't easily >> influence the loading order of components and a change in the order will >> change the behavior of the program. The fix in 22337 solves the exception >> but not the unpredictable behavior. If the link dataset and connection are >> loaded before the dblookup it will open the dataset, else it won't. So your >> program can start up with (some) dblookups pre-loaded or not. >> >> We shouldn't support users counting on unpredictable behavior. >> > Totally agreed. +1 Marcos Douglas

+2 ;-)

Me too.

Antonio.

Show quoted text 6 lines
-- _______________________________________________ Lazarus mailing list http://lists.lazarus.freepascal.org/mailman/listinfo/lazarus

Reply by email

Luiz Americo Pereira Camara
to Ludo Brands: So we have also Michael's vote for dropping the current default dblookup behavior :) Ludo
Ludo Brands wrote 7 lines
>> Default false, I hope. Somebody that wants shaky behavior > should do so >> willingly and "sign" for it. > If that makes you happy, why not ? :) > So we have also Michael's vote for dropping the current default dblookup behavior :)

More then changing the default behavior i propose removing altogether because:

1) The other option will still be broken and those that use it will expect to work properly. If a bug report is added complaining of such bugs we cannot say "Dont use OpenLookupDataset as True since can lead to problems like this". We will have to fix anyway.
2) The code will be significantly cleaned easing maintainability. To support this feature a temporary global flag is added leading to ugly code.
3) Fixing the user projects that relies on that is pretty straight forward
4) Currently LookupCache is ignored and work always as if was True (i plan to fully implement later). To implement it with OpenLookupDataset = True and the lookup dataset closed we would need to open it every time a lookup is needed. This is how was implemented earlier. See TDBLookup.ListFieldValue at
http://svn.freepascal.org/cgi-bin/viewvc.cgi/trunk/lcl/include/dblookup.inc?view=markup&revision=28158&root=lazarus

Luiz

Reply by email