Jackie-Jiang commented on a change in pull request #7638:
URL: https://github.com/apache/pinot/pull/7638#discussion_r736720511



##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java
##########
@@ -835,6 +836,7 @@ public static ForwardIndexCreator 
getRawIndexCreatorForSVColumn(File file, Chunk
             writerVersion);
       case STRING:
       case BYTES:
+        //TODO: Need to support MV here

Review comment:
       Revert the change in this class. We have a different method for MV 
support

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/LoaderUtils.java
##########
@@ -70,9 +71,14 @@ private LoaderUtils() {
             columnMetadata.getTotalNumberOfEntries(), 
columnMetadata.getBitsPerElement());
       }
     } else {
-      DataType dataType = columnMetadata.getDataType();
-      return dataType.isFixedWidth() ? new 
FixedByteChunkSVForwardIndexReader(dataBuffer, dataType)
-          : new VarByteChunkSVForwardIndexReader(dataBuffer, dataType);
+      if (columnMetadata.isSingleValue()) {
+        DataType dataType = columnMetadata.getDataType();
+        return dataType.isFixedWidth() ? new 
FixedByteChunkSVForwardIndexReader(dataBuffer, dataType)
+            : new VarByteChunkSVForwardIndexReader(dataBuffer, dataType);
+      } else {
+        //TODO: Implement MV FixedByte Forward Index reader

Review comment:
       This is already implemented. You may rebase the latest master

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java
##########
@@ -87,6 +90,9 @@
 public class TextIndexHandler implements IndexHandler {
   private static final Logger LOGGER = 
LoggerFactory.getLogger(TextIndexHandler.class);
 
+  //TODO: Find a better way to allocate this
+  private static final int MAX_NUMBER_OF_VALUES = 100;

Review comment:
       This can be read from the `columnMetadata`

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java
##########
@@ -120,6 +120,26 @@ public void add(String document) {
     }
   }
 
+  @Override
+  public void add(String[] documents) {
+    Document docToIndex = new Document();
+
+    // Whenever multiple fields with the same name appear in one document, 
both the
+    // inverted index and term vectors will logically append the tokens of the
+    // field to one another, in the order the fields were added.
+    for (String document : documents) {
+      docToIndex.add(new TextField(_textColumn, document, Field.Store.NO));
+      docToIndex.add(new StoredField(LUCENE_INDEX_DOC_ID_COLUMN_NAME, 
_nextDocId++));

Review comment:
       I think all the documents should be stored under the same pinot docId?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to