[ https://issues.apache.org/jira/browse/LUCENE-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12757007#action_12757007 ]
Uwe Schindler edited comment on LUCENE-1919 at 9/18/09 12:01 AM: ----------------------------------------------------------------- Good morning all together! What a nice day and then this problem :-) Here is my solution for the problem: Michael and me never thought about mixing the old and very old API as consumer, which seems to be used. The problem is now, that the behaviour changed for calling next(). The original 2.4.1 code looks the following: {code} public Token next() throws IOException { final Token reusableToken = new Token(); Token nextToken = next(reusableToken); if (nextToken != null) { Payload p = nextToken.getPayload(); if (p != null) { nextToken.setPayload((Payload) p.clone()); } } return nextToken; } {code} The difference is, that a new token is created *before* the call to the reusable next() methods (incrementToken() is somehow reuseable, too). The attached patch, restores exactly this functionality, but also for incrementToken(). It also removes an unneeded assignment to the delegate in the case of next(Token) delegating to next() (which is also a bug, because it makes the token no longer private for the caller - it can be overridden by a later call to incrementToken()) - sorry for that. Tokens generated by next() must be always private and not shared or reused as reusableToken for later next/incrementToken calls by the API. This is the problem of Robert's patch. The full private token (which was cloned before) is then also available as delegate for later calls to incrementToken() (for next(Token) the delegate is replaced, as Robert noticed, so no problem here). The wrapper around incrementToken() does the following: - save the current delegate - replace delegate by a new Token (just like the old next() in 2.4.1) - call incrementToken and assign to nextToken variable - restore the reusable delegate - after that all goes like for next(Token) Simply said: the next() wrapper is completely decoupled and always uses a completely private (new) Token instance. I am not sure, why this payload cloning code is in 2.4.1, but I moved it here, too. I think it is because of some old bug, where a payload was assigned in next(Token), that was also shared by the TokenStream itsself between more than one tokens. Using this code, the Token is for sure full private (and even not reused later as before). Using this patch, you should now even be able to mix all three APIs in one filter/consumer - but I still would'nt do this :-) was (Author: thetaphi): Here is my solution for the problem. Michael and me never thought about mixing the old and very old API, which seems to be used. The problem is now, that the behaviour changed for calling next(). The original 2.4.1 code looks the following: {code} public Token next() throws IOException { final Token reusableToken = new Token(); Token nextToken = next(reusableToken); if (nextToken != null) { Payload p = nextToken.getPayload(); if (p != null) { nextToken.setPayload((Payload) p.clone()); } } return nextToken; } {code} The difference is, that a new token is created *before* the call to the reusable next() methods (incrementToken() is somehow reuseable, too). The attached patch, restores exactly this functionality, but also for incrementToken(). It also removes an unneeded assignment to the deleget in the case of next(Token) delegating to next() (which is also a bug, because it makes the token no longer private for the caller - it can be overridden by a later call to incrementToken()). Tokens generated by next() must be always private and not shared or reused as reusableToken for later next/incrementToken calls by the API. This is the problem of Robert's patch. The full private token (which was cloned before) is then also available as delegate for later calls to incrementToken() (for next(Token) the delegate is replaced, as Robert noticed, so no problem here). The wrapper around incrementToken() does the following: - save the current delegate - replace delegate by a new Token (just like the old next() in 2.4.1) - call incrementToken and assign to nextToken variable - restore the reusable delegate - after that all goes like for next(Token) Simply said: the next() wrapper is completely decoupled and always uses a completely private (new) Token instance. I am not sure, why this payload cloning code is in 2.4.1, but I moved it here, too. I think it is because of some old bug, where a payload was assigned in next(Token), that was also shared by the TokenStream itsself between more than one tokens. Using this code, the Token is for sure full private (and even not reused later as before). Using this patch, you should now even be able to mix all three APIs in one filter/consumer - but I still would'nt do this :-) > Analysis back compat break > -------------------------- > > Key: LUCENE-1919 > URL: https://issues.apache.org/jira/browse/LUCENE-1919 > Project: Lucene - Java > Issue Type: Bug > Reporter: Yonik Seeley > Assignee: Uwe Schindler > Fix For: 2.9 > > Attachments: LUCENE-1919.patch, LUCENE-1919.patch, LUCENE-1919.patch, > LUCENE-1919.patch > > > Old and new style token streams don't mix well. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. --------------------------------------------------------------------- To unsubscribe, e-mail: java-dev-unsubscr...@lucene.apache.org For additional commands, e-mail: java-dev-h...@lucene.apache.org