Opened 14 years ago

Closed 21 months ago

#523 closed defect (fixed)

Migrate from XS Search::Xapian to SWIG version

Reported by: Olly Betts Owned by: Olly Betts
Priority: normal Milestone: 1.4.23
Component: Search::Xapian Version: git master
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Operating System: All

Description (last modified by Olly Betts)

This ticket is to track the issues remaining to solve before the SWIG Perl bindings are a complete replacement for the XS ones.

  • ~"$it" doesn't work in SWIG, while in XS it dereferences (for TermIterator and ValueIterator at least). I did some digging last year and it seemed there was some problematic interaction with eq/ne which I didn't get to the bottom of.~
  • ~"Stringify" is also missing for subclasses of Error (which in the XS version I'm using in 1.2.4 to give backwards compatibility for cases which were still throwing string exceptions in 1.2.3).~
  • ~XS version has simple anon-sub wrappers for MatchDecider and ExpandDecider. SWIG doesn't support directors for Perl in current releases (though there's work on a branch). But perhaps we can support these simple wrappers without needing directors.~
  • ~The SWIG-generated version is fussier about what sort of scalars you pass - for example, if an integer is expected, passing a string containing an integer or floating point number doesn't work, at least in some cases.~ Now documented at least.

One option is to deprecate the stringify stuff and make the switch at the start of the next release series.

Change History (22)

comment:1 by Olly Betts, 14 years ago

Milestone: 1.2.x1.3.0

Doing this for the next release series seems to make most sense. Now we've made a release branch for 1.2.x, I've removed the XS Search::Xapian from trunk.

comment:2 by Olly Betts, 14 years ago

It would be nice to wrap iterators in a more natural way for Perl - this article gives a good approach which would probably work:

http://www.perl.com/pub/2005/06/16/iterators.html

comment:3 by Olly Betts, 14 years ago

Description: modified (diff)

comment:4 by Olly Betts, 13 years ago

Milestone: 1.3.01.3.x

Backported r16227

comment:5 by Olly Betts, 10 years ago

Milestone: 1.3.x1.3.5

comment:6 by Olly Betts, 9 years ago

Milestone: 1.3.51.3.x
Version: SVN trunkgit master

I talked through these issues at a Wellington PM meeting a while ago and have notes from that, but I think this is going to take a while to work through properly.

I'm pondering if we should release 1.4.0 with the Perl bindings clearly labelled as "still experimental", and then resolve this in early 1.4.x.

Moving to 1.3.x for now.

comment:7 by Olly Betts, 9 years ago

SWIG now supports directors for Perl, though taking a code ref (which would typically be an anonymous subroutine) would be a nicer Perl API for the simple functors.

comment:8 by Olly Betts, 9 years ago

Milestone: 1.3.x1.4.1

Postpone until 1.4.1

comment:9 by Olly Betts, 8 years ago

Milestone: 1.4.11.4.3

Search::Xapian 1.2.24.0 will be compatible with xapian-core 1.4.x so this is a little less urgent.

comment:10 by Olly Betts, 8 years ago

Milestone: 1.4.31.4.4

Ticket retargeted after milestone closed

comment:11 by Olly Betts, 8 years ago

Milestone: 1.4.41.4.5

Ticket retargeted after milestone closed

comment:12 by Olly Betts, 7 years ago

Milestone: 1.4.51.4.6

Ticket retargeted after milestone closed

comment:13 by Eric Wong, 6 years ago

Xapian::PostingIterator lacks "->inc" sub from Search::Xapian::PostingIterator; so I need to rely on "++" which I find confusing (overloading confuses me :x)

Also, I could not find Xapian::ENQ_ASCENDING, since I rely on Search::Xapian::ENQ_ASCENDING.

comment:14 by Olly Betts, 5 years ago

Xapian::PostingIterator lacks "->inc" sub from Search::Xapian::PostingIterator; so I need to rely on "++" which I find confusing (overloading confuses me :x)

Thanks for reporting. I've pushed fde36e74d72b36b19dea6eb83b70ee1311423c40 to fix that, and will backport for 1.4.16.

comment:15 by Olly Betts, 5 years ago

I could not find Xapian::ENQ_ASCENDING, since I rely on Search::Xapian::ENQ_ASCENDING.

This is wrapped as $Xapian::Enquire::ASCENDING (which better matches the C++ API).

But we try to provide compatibility with Search::Xapian so I've pushed d7776b1206fb6fd33a715a38b87678e14ecadd3f to add aliases for these.

comment:16 by Olly Betts, 5 years ago

I debugged what happens when stringify overloads are added back in.

The problem is that SWIG internally adds objects to a hash using the object as the hash key. Perl stringifies the hash key to get a string to hash, and that doesn't work out well if the stringify overloads aren't a unique identifier for the C++ object (which the Search::Xapian overloads aren't).

It'd be better if SWIG didn't rely on stringifying objects like this, but I'm not sure the Search::Xapian stringify overloads are a good idea either.

comment:17 by Olly Betts, 5 years ago

Description: modified (diff)
Milestone: 1.4.61.5.0
Status: newassigned

The convert to integer overload on PositionIterator also seems to cause problems, though I haven't tried to dig into why.

I think these magic implicit method calls on conversions to string and integer are too confusing, and it's better not to support them. This is incompatible with Search::Xapian, but we can document this and it's easy to write code which works with both versions - just make the method call explicit.

comment:18 by Olly Betts, 5 years ago

We should also document a good way to load whichever of Search::Xapian or Xapian is available, rather than everybody having to invent their own way. Perhaps:

BEGIN {  
    eval {  
        require Xapian; Xapian->import(':all');  
    };  
    if ($@) {  
        require Search::Xapian; Search::Xapian->import(':all');  
    }  
}

comment:19 by Olly Betts, 5 years ago

I've improved the documentation of the newer bindings and added a compatibility section which shows the require code to load either, and at least document the increased fussiness about string vs integer scalars.

I think the main thing left is wrapping MatchDecider and ExpandDecider to call Perl functions.

comment:20 by Olly Betts, 5 years ago

Description: modified (diff)

comment:21 by Olly Betts, 5 years ago

Description: modified (diff)

I've added support for passing a Perl sub for simple functor classes in d5d26eb54bfb68e1674b8f0e314de29e81e16b1d.

SWIG now supports directors for Perl so we should be able to support subclassing, but accepting a Perl sub as an alternative still seems useful for simple cases (it'd be nice to support for C++ too).

I think the coding work for this ticket is now done (unless further incompatibilities are found), but we should make sure the documentation reflects the updated status of the SWIG-generated Perl bindings.

comment:22 by Olly Betts, 21 months ago

Milestone: 1.5.01.4.23
Resolution: fixed
Status: assignedclosed

[b2c5a064bb28088f024ac1526c429e5e0876dee3] updated some wording about the status of these bindings.

I'm working through putting the docs for these on the website too, though probably that won't be completed until the next 1.4.x release as the docs there are built from release tarballs.

I'll also update the getting started guide.

Note: See TracTickets for help on using tickets.