gortiz commented on code in PR #10184: URL: https://github.com/apache/pinot/pull/10184#discussion_r1138794547
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/text/LuceneFSTIndexCreator.java: ########## @@ -21,11 +21,14 @@ import java.io.File; Review Comment: We already chat about that, but I want to drop these lines here in order to inform the others: FST index and Text index are two completely different things that by index String columns. In some parts of the code we consider them two flavours of the same thing and in other we consider them different things. In my opinion, they are actually very different things, they actually change the semantics of the queries when one or the other index the column (match behaves different!). I would like to split them even more in this PR, but I tried to apply the less possible changes (which is funny to say when the PR changes 10k lines!). In this particular case, the IndexCreators are completely different. The older implementation of `SegmentColumnarIndexCreator` stored one map for text index creators and one map for fst index creators. The method `SegmentColumnarIndexCreator.addRow` did called `add` for each text index creator but did not call `add` for fst index creators. That is why in this PR both FSTIndexCreators implement the add methods as a NOOP. I insist that I strongly believe that we should totally split FST and text indexes, but I didn't tried to do that in this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
