You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucy.apache.org by Peter Karman <pe...@peknet.com> on 2011/05/01 05:01:16 UTC
[lucy-dev] FType_Equals
This change:
================================
r1085159 | marvin | 2011-03-24 17:02:51 -0500 (Thu, 24 Mar 2011) | 4 lines
LUCY-138 ftype_equals.patch
Have FType_Equals() return false whenever classes for "self" and "other" don't
match.
================================
is causing some of my existing code to break.
The change was this:
Index: core/Lucy/Plan/FieldType.c
===================================================================
--- core/Lucy/Plan/FieldType.c (revision 1085158)
+++ core/Lucy/Plan/FieldType.c (revision 1085159)
@@ -87,6 +87,7 @@
{
FieldType *evil_twin = (FieldType*)other;
if (evil_twin == self) return true;
+ if (FType_Get_VTable(self) != FType_Get_VTable(evil_twin)) return false;
if (self->boost != evil_twin->boost) return false;
if (!!self->indexed != !!evil_twin->indexed) return false;
if (!!self->stored != !!evil_twin->stored) return false;
When I add some verbosity to the resulting error message:
Index: core/Lucy/Plan/Schema.c
@@ -108,7 +108,7 @@
// If the field already has an association, verify pairing and return.
if (existing) {
if (FType_Equals(type, (Obj*)existing)) { return; }
- else { THROW(ERR, "'%o' assigned conflicting FieldType", field); }
+ else { THROW(ERR, "'%o' cannot be %o because already assigned \
conflicting FieldType %o", field, existing, type); }
}
if (FType_Is_A(type, FULLTEXTTYPE)) {
I get this, for example:
'foo' cannot be Lucy::FieldType::FullTextType@0x000000010190d870 because already
assigned conflicting FieldType Lucy::Plan::FullTextType@0x000000010193bfe0
All of which makes me think that the comparison being done isn't really between
classes, as the commit message claims, because the objects compared are, in
fact, instances of the same class.
What am I missing?
--
Peter Karman . http://peknet.com/ . peter@peknet.com
Re: [lucy-dev] FType_Equals
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Sun, May 01, 2011 at 06:46:54AM -0500, Peter Karman wrote:
> > If you have conflicting field type classes in your active code, you should fix
> > that and hopefully the problem will go away.
>
> perfect. that was it.
Cool, glad that worked out. :)
With r1098463, the bogus compatibility code supporting the following
legacy-classes-which-never-existed has been ripped out:
Lucy::FieldType::FullTextType # should be Lucy::Plan::FullTextType
Lucy::FieldType::StringType # should be Lucy::Plan::StringType
Lucy::FieldType::BlobType # should be Lucy::Plan::BlobType
Lucy::Schema # should be Lucy::Plan::Schema
Attempting to use any of them will now produce an immediate crash.
I had overlooked that compat code when migrating the namespace.
Marvin Humphrey
Re: [lucy-dev] FType_Equals
Posted by Peter Karman <pe...@peknet.com>.
Marvin Humphrey wrote on 4/30/11 11:37 PM:
> They aren't quite the same -- they both start with "Lucy" and end with
> "FullTextType", but in between, one has "FieldType" and the other has "Plan".
ah! thanks. better eyes than mine.
>
> If you have conflicting field type classes in your active code, you should fix
> that and hopefully the problem will go away.
perfect. that was it.
thanks.
--
Peter Karman . http://peknet.com/ . peter@peknet.com
Re: [lucy-dev] FType_Equals
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Sat, Apr 30, 2011 at 10:01:16PM -0500, Peter Karman wrote:
> This change:
> ================================
> r1085159 | marvin | 2011-03-24 17:02:51 -0500 (Thu, 24 Mar 2011) | 4 lines
>
> LUCY-138 ftype_equals.patch
> Have FType_Equals() return false whenever classes for "self" and "other" don't
> match.
> ================================
>
> is causing some of my existing code to break.
Mmm, yes, I can see why this might be happening. Some of our backwards
compatibility stubs are not providing enough backwards compatibility.
That changeset implements an important bugfix, though. We discovered the
problem it addresses after corrupting an index because we didn't detect that a
field type had changed from one subclass to another and throw an error before
the damage was unrecoverable. :(
Without that patch, Lucy will let you change a field to a different type --
say one that uses a different posting format -- so long as the other tests
within FType_Equals return true. That's bad, because as soon as the indexing
session completes, you start trying to read from old segments using the wrong
posting codec and Lucy crashes.
> 'foo' cannot be Lucy::FieldType::FullTextType@0x000000010190d870 because already
> assigned conflicting FieldType Lucy::Plan::FullTextType@0x000000010193bfe0
>
>
>
> All of which makes me think that the comparison being done isn't really between
> classes, as the commit message claims, because the objects compared are, in
> fact, instances of the same class.
>
> What am I missing?
They aren't quite the same -- they both start with "Lucy" and end with
"FullTextType", but in between, one has "FieldType" and the other has "Plan".
When we moved FullTextType from KinoSearch::FieldType::FullTextType to
KinoSearch::Plan::FullTextType, we installed a compatibility stub subclass at
the old location so that code like this would continue to work:
my $type = KinoSearch::FieldType::FullTextType->new(%args);
However, the classes aren't exactly the same -- the FieldType variant now
subclasses the Plan variant -- and after the commit in question,
FType_Equals() won't treat them as equivalent.
Probably the best solution for Lucy is to rip out all the compatibility stub
subclasses that were inherited from KinoSearch. You should make sure that all
new code uses the Lucy::Plan::* variant.
If you have conflicting field type classes in your active code, you should fix
that and hopefully the problem will go away.
If you don't, then you're hitting a problem that arises because within the
schema file, the class name for stock FullTextType fields is not recorded --
they are just stored as "type":"fulltext". This is a little complicated to
explain and it will go away if we remove the compat stubs, so I'll hold off
on the long version until we see whether Plan A works for you.
Marvin Humphrey