[Monetdb-developers] [Monetdb-pf-checkins]pathfinder/compiler/core core.c, , 1.68, 1.69 coreopt.brg, , 1.77, 1.78 fs.brg, , 1.78, 1.79 simplify.brg, , 1.44, 1.45

Jan Flokstra flokstra at cs.utwente.nl
Fri Nov 6 11:09:49 CET 2009

On Thursday 05 November 2009 23:45, Jan Rittinger wrote:
> Hi Anandeshwar,
> thanks a lot for extending pathfinder with full-text functionality.

Hi Jan,

Thanks for your comments. Anand had a development version on his PC for half a 
year which survived a couple of disk crashes and a couple of nasty 
integration problems because the compiler code changed. Because Anand moves 
on soon we decided to check his stuff in. We did a thorough test of his 
changes and after his changes did not break any test from the entire 
Pathfinder test collection we decided it was time to check his stuff in. We 
are aware we changed the original compiler stuff at a couple of places but 
most of them were inevitable.

> I'm however a little bit taken by surprise and your checkin in my
> opinion leaves some questions open:

> - Why did you change the translation of PFcore_some in core.c?
Could this be and old re-checkin of something changed long time ago. It could 
have been accidently reintroduced with one of the reintegration or crash 
recoveries. This is indeed no change necessary for a fulltext construction.

> - Why did this rewrite touch any file in the algebra folder? -
There were some score handling issues which could be handled easyer in 
algebra. When the complete score handling and propagation in core to algebra 
is finished these algebra changes can be removed. If scores are not used the  
changes do not do anyting and when used they will not break anything. 

> Whitespace removal is not ftcontains support as your message suggests.
I do not understand which message you mean? It looks like a cryptic way of you 
to say the indent and whitespace layout of files are changed unnecessarily.

> Additionally disabling consistency checks without an explanation isn't
> a nice thing.
This is indeed an error. A lot of checks have been disabled during development 
but they should all have be switched 'on' again. I suppose you mean the 
PFprop_type_of_() check where the PFoops has been disabled. There should have 
been an 'incomplete' message there.

> - Why did you duplicate the translation of a for loop? - Isn't the
> score just an optional modification like the positional variable? I at
> least don't like this code duplication as it will make code
> maintenance a lot harder.
> - As far as I can oversee you did break the compositionality of the
> compilation scheme. Now the different rules provide different result
> schemas.

You are right about the last two points but the way I understand it it is 
necessary to do it this way because the ftcontains clause changes the 
semantics of the for loop. I heard Stefan Klinger also complain a lot about 
the horrible semantics of the xquery fulltext for loops. So the new rules 
indeed break the compositionality but they are supposed to :-(

> I think (mainly because of the last issue) this checkin should better
> go into a separate branch instead of the CVS HEAD until the
> translation is more mature.

We could go into a separate branch but it looks this fulltext project will be 
a long project. We will have a separate branch for year with lots of overhead 
for the CSV maintainers and our mailboxes. I think 'ifdeffing' most of the 
fulltext stuff will give a better overview what has changed because of xquery 
fulltext and what are the real problems.  Also compiler developpers have a 
better overview when they change things where fulltext isssues are involved,
	Anandeshwar Singh  & Jan Flokstra.
> Jan

More information about the developers-list mailing list