Hi Shai,
Thanks for the suggestions! About your points: 1) This is really wrong, we can easily fix it for 3.1. Lucene 3.0 is already in the vote phase and 2.9x is also already out. 2) Maybe the explanation is not so good. This text comes especially from the 2.9 old to new TS migration. The current indexer normally uses all attributes and the default Token implementation implements all six (in 2.9). We had a lot of TokenStreams in core that used getAttribute without checking, if the attribute is really there. With Token/TokenWrapper as implementation, this was always ok, but as soon as you switched to new API only, the attributes were implemented by single instances, so after adding a TermAttribute you were not be sure, to also have a PosIncr attribute. Even the indexer of Lucene had that problem. So for the basic six attributes you can always use addAttribute(), because if you not add the attribute in the instantiation of the stream, all attributes are added by the indexer. This applies to TermAttribute, PosIncrementAttribute and OffsetAttribute (if TV are enabled). So if the chain does not add them, they are added by the indexer (consumer). Because all attributes have a default value, this is no problem and the indexer code is even faster (e.g. it does not make sense to every time check for a PositionIncrementAttribute, as reading the default int value of 1 is read in the same speed, even faster without the extra null check. And with current index format it is always tracked and indexed). If you have own attribute, it may correctly make sense to check with hasAttribute() and throw IAE. But after this check, you can also use addAttribute() to get the reference to the attribute (there were a discussion about removing getAttribute at all). getAttribute and addAttribute have the same speed when fetching an already existing attribute. So it is up to yours, how you would implement your tokenstreams, that are just hints for beginners. The thing with hasAttribute() and addAttribute() is also mentioned in the docs a few lines down at the "optimization" part. 3) Attribute interfaces cannot be cloned (the Attribute interface has no clone()). You can only clone implementations (AttributeImpl), but it may happen that you not only clone the TermAttribute, but some other attributes also (e.g. if you use Token as impl, like 2.9 does by default for backwards compatinility). Because of that, attribute instances cannot be simply cloned, only their implementing classes. What you generally should do (see caching TokenFilter): use AttributeSource.captureState() to capture all current attributes in the filter chain. In incrementToken(), you can restore these captured states again into the existing attributes. Such states can be handles in local instance variables or Lists<State> etc. The second point I do not understand: Consumers always call addAttribute (maybe hasAttribute/getAttribute) exactly one time before consuming! So they only have the current attribute instance, which can never change during the lifetime of a TokenStream chain. Also all TokenFilters/TokenStreams/Tokenizers use the same instances! Because of this the TS chain works. If every TokenStream would have its own attributes, they could not communicate! The important thing is, that you cannot relay on the fact that a chain may contain attributes or not, because even if you created a the Tokenizer and one TokenFilter in the chain and checked for hasAttribute() in the ctor of the filter, it may happen, that the third TokenFilter adds a new Attribute, which is then suddenly available (but the second filter does not see it - ok, it may not need to know about it, because the Tokenizer will never set it). Because of that its always best to use addAttribute, if you are "interested" on an attribute. If it does not exist, you just get default values (like termLength()==0 for TermAttribute, posIncr==1,.). Hope that explains your questions. Uwe ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de eMail: u...@thetaphi.de _____ From: Shai Erera [mailto:ser...@gmail.com] Sent: Sunday, November 22, 2009 9:37 AM To: java-dev@lucene.apache.org Subject: Problematic documentation in o.a.l.analysis.package.html Hi I've read the analysis package.html and I found two issues: 1) The code sample under Invoking the Analyzer is broken. It calls incrementToken() but inside the while it prints 'ts' (which is TokenStream) and then do "t = ts.next()", which no longer works. That's an easy fix, so I don't think a JIRA issue is needed. 2) The documentation specifies that "Even consumers of TokenStreams should normally call addAttribute() instead of getAttribute(), because it would not fail if the TokenStream does not have this Attribute". IMO this is wrong and will give the wrong impression about how this API should be used. What if the TokenStream does not care about this attribute? It will not fill it with any information. The example with LengthFilter which calls addAttribute instead of has/getAttribute is a good one regarding why you shouldn't just call addAttribute. LegthFilter relies on the given TokenStream to fill TermAttribute with some information, so that it can later filter out terms of length < threshold. But what if I create a LengthFilter and give it a TokenStream which creates just PartOfSpeechAttribute? Or output terms that are not TermAttribute? Obviously it would be silly for me to do it, but no one restricts me from doing so. LengthFilter should either document that it expects TermAttribute to be returned from the input TokenStream, or better yet, enforce it in the constructor --> if you pass a TokenStream that does not return TermAttribute, throw an IllegalArgumentException. But anyway, the current documentation is, IMO, wrong and may lead to wrong impression. I don't know if this warrants a larger issue to investigate all the current TokenFilters and validate the input TokenStream. In my filters, I enforce the existence of a certain attribute. If I've misunderstood something, please correct me. 3) I think it would help if there will be some documentation/example about how TokenFilters are expected to process an Attribute before they return it. For example, if I have a TokenFilter which processes a certain TermAttribute by returning two other TermAttributes, then according to my understanding, upon calling incrementToken() it should: 3.1) If first call, clone the TokenStream's TermAttribute in an instance variable. Then process it and store in the TokenStream's TermAttribute the first TA it should return. 3.2) If second call, process it again and store the second TA in the TokenStream's TermAttribute. That's because the consumer will call incrementToken and then getAttribute. That getAttribute will return the TokenStream's attribute and not the filter's. I think I've read it somewhere, but it doesn't appear in this package.html. Shai
