Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Thu, Oct 28, 2010 at 8:27 AM, Hanno Schlichting wrote: > On Wed, Oct 27, 2010 at 8:56 PM, Jim Fulton wrote: ... >> As I said earlier, in 3.11, it will be an error to use an object with >> default comparison as a key, but loading state with such objects will >> only warn. Looking at this a bit more. I decided I'll only error when updating a BTree or set using __setitem__ or add. So even in 3.11, you won't get an error when loading an object or using an invalid key in a query method. You will still get a warning, > I agree with them being useful. And once I'm not busy at the > conference, I'll give it more proper testing. Thanks. Note that to get at the version you were playing with, you'll need to select a specific svn revision, as I reverted the relevant changes from the 3.10 branch. I exhausted by time budget for this and it may take a few days before I get changes back on the 3.10 branch (and refine my changes on the trunk). Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 8:56 PM, Jim Fulton wrote: > On Wed, Oct 27, 2010 at 1:59 PM, Hanno Schlichting wrote: > > I suppose that depends on the application. Was the use of None > intentional, or the result of sloppy coding? I'd have to check the code. I expect it to be sloppy coding. > I'll make a 3.10.1 release without it and a 3.10.2a1 release with it. Thanks a lot for this! > I do think these warnings are beneficial, possibly wildly so. :) > As I said earlier, in 3.11, it will be an error to use an object with > default comparison as a key, but loading state with such objects will > only warn. I agree with them being useful. And once I'm not busy at the conference, I'll give it more proper testing. Thanks, Hanno ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 5:51 PM, Jim Fulton wrote: > I'm inclined to treat the use of the comparison operator inherited > from object in BTrees to be a bug. I plan to fix this on the > trunk. > > I'm tempted to fix this in 10.1. This change would make it impossible > to add keys to BTrees or buckets or to add items to BTree-based > sets if the key or items inherits it's comparison from object. This > would only apply to instances of new-style classes, including > persistent objects. (It wouldn't affect old-style-class instances, > which are too hard to introspect.) Of course, it also wouldn't protect anybody from objects with broken comparisons or objects that base reasonably sane comparison on comparison of sub objects with bad comparison, such as: (object(), ) :) I do think it would catch a common class of bug involving using objects as keys. Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 1:59 PM, Hanno Schlichting wrote: > On Wed, Oct 27, 2010 at 6:45 PM, Jim Fulton wrote: >> If not, I wonder if the existing indexes have some bad values in >> them that are triggering this somehow. The relevant check is being done >> when loading state. I bet you have some bad keys (e.g. None) in your >> data structures. Could you check that? If this is what's happening, then >> I think the warning is useful. For 3.11 though (aka trunk) I'll >> rearrange things >> so the check isn't performed when loading state. > > Aha. I haven't looked into all the cases, but I did find a None value > in the two examples I checked. > > So in an OOTree with string keys, None is considered invalid Short answer: yes None's ordering wrt strings can change, and I believe it has in the past. It compares solely on address. In Python 3, it is an error to compare None with something else. Using None as a BTree key seems like a very bad idea. > and would need to be an empty string? I suppose that depends on the application. Was the use of None intentional, or the result of sloppy coding? I'm thinking that this change is scary enough that I'm going to separate it from 3.10.1, especially since 3.10.1 has some other bug fixes that are pretty important. I'll make a 3.10.1 release without it and a 3.10.2a1 release with it. I do think these warnings are beneficial, possibly wildly so. :) As I said earlier, in 3.11, it will be an error to use an object with default comparison as a key, but loading state with such objects will only warn. Thanks much for trying this out! Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 6:45 PM, Jim Fulton wrote: > If not, I wonder if the existing indexes have some bad values in > them that are triggering this somehow. The relevant check is being done > when loading state. I bet you have some bad keys (e.g. None) in your > data structures. Could you check that? If this is what's happening, then > I think the warning is useful. For 3.11 though (aka trunk) I'll > rearrange things > so the check isn't performed when loading state. Aha. I haven't looked into all the cases, but I did find a None value in the two examples I checked. So in an OOTree with string keys, None is considered invalid and would need to be an empty string? Thanks! Hanno ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Wed, Oct 27, 2010 at 9:17 AM, Hanno Schlichting wrote: > On Mon, Oct 25, 2010 at 11:51 PM, Jim Fulton wrote: >> I'm inclined to treat the use of the comparison operator inherited >> from object in BTrees to be a bug. I plan to fix this on the >> trunk. > > Did you mean to throw warnings for simple built-in types? No. > I'm now getting warnings for simple strings and ints, I'd expect > tuples as well. That's odd. I'm not. What platform and Python version? Can you give some examples? > All of these do inherit from object and use the > default __cmp__. I'm not sure what you mean by "these", but strings, int, and tuples certainly don't use the default comparison. If they did, sorting on them would be useless. > But their hash implementation used in the default > __cmp__ should be safe to use. Strings, ints and tuples don't use hash for comparison. Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 11:51 PM, Jim Fulton wrote: > I'm inclined to treat the use of the comparison operator inherited > from object in BTrees to be a bug. I plan to fix this on the > trunk. Did you mean to throw warnings for simple built-in types? I'm now getting warnings for simple strings and ints, I'd expect tuples as well. All of these do inherit from object and use the default __cmp__. But their hash implementation used in the default __cmp__ should be safe to use. Hanno ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 6:34 PM, Marius Gedminas wrote: ... > Or perhaps make it emit DeprecationWarnings, but continue working. Then > make it a fatal error in the next minor/major release. I like this idea. I think I'm going to use UserWarning because it isn't disabled in Python 2.7 afaik. Otherwise, I would liie deprecation warning because it suggest that things are bad and going to get worse. :) Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On 25/10/2010 23:34, Marius Gedminas wrote: > Or perhaps make it emit DeprecationWarnings, but continue working. Then > make it a fatal error in the next minor/major release. Well, not a DeprecationWarning... Is there a DataLossWarning? Still, +1 on the warning followed by exception in 2 releases... Chris -- Simplistix - Content Management, Batch Processing & Python Consulting - http://www.simplistix.co.uk ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
> Or perhaps make it emit DeprecationWarnings, but continue working. Then > make it a fatal error in the next minor/major release. > +1 Pedro -- José Pedro Ferreira Indico Team IT-UDS-AVC 513-R-0042 CERN, Geneva, Switzerland ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 11:14:21PM +0100, David Glick wrote: > On 10/25/10 11:07 PM, Jim Fulton wrote: > > On Mon, Oct 25, 2010 at 5:58 PM, David Glick > > wrote: > >> On 10/25/10 10:51 PM, Jim Fulton wrote: > >>> I'm inclined to treat the use of the comparison operator inherited > >>> from object in BTrees to be a bug. I plan to fix this on the > >>> trunk. > >>> > >>> I'm tempted to fix this in 10.1. This change would make it impossible > >>> to add keys to BTrees or buckets or to add items to BTree-based > >>> sets if the key or items inherits it's comparison from object. This > >>> would only apply to instances of new-style classes, including > >>> persistent objects. (It wouldn't affect old-style-class instances, > >>> which are too hard to introspect.) > >>> > >>> Thoughts? I like this. > >> The motivation here is that the comparison inherited from object > >> compares the objects' memory locations, which is not stable beyond > >> deactivation for persistent objects, and therefore not suitable for use > >> as a BTree key. Correct? > > Yup. Thanks for clarifying. > > > >> Preventing people from making that mistake > >> sounds like a good thing to me. > > It will likely reveal that current applications are broken. > > This will likely cause some pain. Where previously, > > apps simply lost data, now they'll error. I'm afraid that > > this might be too disruptive for a bug-fix release. > Maybe add it as an option defaulted to off? So that app developers who > want to check whether they have this problem can easily do so, and have > the extra protection once they've fixed any issues? Or perhaps make it emit DeprecationWarnings, but continue working. Then make it a fatal error in the next minor/major release. Marius Gedminas -- Include me out. signature.asc Description: Digital signature ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On 10/25/10 11:07 PM, Jim Fulton wrote: > On Mon, Oct 25, 2010 at 5:58 PM, David Glick > wrote: >> On 10/25/10 10:51 PM, Jim Fulton wrote: >>> I'm inclined to treat the use of the comparison operator inherited >>> from object in BTrees to be a bug. I plan to fix this on the >>> trunk. >>> >>> I'm tempted to fix this in 10.1. This change would make it impossible >>> to add keys to BTrees or buckets or to add items to BTree-based >>> sets if the key or items inherits it's comparison from object. This >>> would only apply to instances of new-style classes, including >>> persistent objects. (It wouldn't affect old-style-class instances, >>> which are too hard to introspect.) >>> >>> Thoughts? >> The motivation here is that the comparison inherited from object >> compares the objects' memory locations, which is not stable beyond >> deactivation for persistent objects, and therefore not suitable for use >> as a BTree key. Correct? > Yup. Thanks for clarifying. > >> Preventing people from making that mistake >> sounds like a good thing to me. > It will likely reveal that current applications are broken. > This will likely cause some pain. Where previously, > apps simply lost data, now they'll error. I'm afraid that > this might be too disruptive for a bug-fix release. Maybe add it as an option defaulted to off? So that app developers who want to check whether they have this problem can easily do so, and have the extra protection once they've fixed any issues? -- David Glick Web Developer davidgl...@groundwire.org 206.286.1235x32 Groundwire: You Are Connected http://groundwire.org Online tools and stratgies for the environmental movement. Sign up for Groundwire News! http://groundwire.org/email-capture ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On Mon, Oct 25, 2010 at 5:58 PM, David Glick wrote: > On 10/25/10 10:51 PM, Jim Fulton wrote: >> I'm inclined to treat the use of the comparison operator inherited >> from object in BTrees to be a bug. I plan to fix this on the >> trunk. >> >> I'm tempted to fix this in 10.1. This change would make it impossible >> to add keys to BTrees or buckets or to add items to BTree-based >> sets if the key or items inherits it's comparison from object. This >> would only apply to instances of new-style classes, including >> persistent objects. (It wouldn't affect old-style-class instances, >> which are too hard to introspect.) >> >> Thoughts? > The motivation here is that the comparison inherited from object > compares the objects' memory locations, which is not stable beyond > deactivation for persistent objects, and therefore not suitable for use > as a BTree key. Correct? Yup. Thanks for clarifying. > Preventing people from making that mistake > sounds like a good thing to me. It will likely reveal that current applications are broken. This will likely cause some pain. Where previously, apps simply lost data, now they'll error. I'm afraid that this might be too disruptive for a bug-fix release. Jim -- Jim Fulton ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] Default comparison considered harmful in BTrees.
On 10/25/10 10:51 PM, Jim Fulton wrote: > I'm inclined to treat the use of the comparison operator inherited > from object in BTrees to be a bug. I plan to fix this on the > trunk. > > I'm tempted to fix this in 10.1. This change would make it impossible > to add keys to BTrees or buckets or to add items to BTree-based > sets if the key or items inherits it's comparison from object. This > would only apply to instances of new-style classes, including > persistent objects. (It wouldn't affect old-style-class instances, > which are too hard to introspect.) > > Thoughts? The motivation here is that the comparison inherited from object compares the objects' memory locations, which is not stable beyond deactivation for persistent objects, and therefore not suitable for use as a BTree key. Correct? Preventing people from making that mistake sounds like a good thing to me. -- David Glick Web Developer davidgl...@groundwire.org 206.286.1235x32 Groundwire: You Are Connected http://groundwire.org Online tools and stratgies for the environmental movement. Sign up for Groundwire News! http://groundwire.org/email-capture ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org https://mail.zope.org/mailman/listinfo/zodb-dev
