Jackie-Jiang commented on code in PR #19897:
URL: https://github.com/apache/pinot/pull/19897#discussion_r4380548550
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java:
##########
@@ -57,7 +59,9 @@
/// and realtime from
[org.apache.pinot.segment.local.realtime.impl.invertedindex.RealtimeLuceneTextIndex]
public class LuceneTextIndexCreator extends AbstractTextIndexCreator {
private static final Logger LOGGER =
LoggerFactory.getLogger(LuceneTextIndexCreator.class);
- public static final String LUCENE_INDEX_DOC_ID_COLUMN_NAME = "DocID";
+ public static final String PINOT_DOC_ID_STORED_FIELD_NAME = "DocID";
+ public static final String PINOT_DOC_ID_DOC_VALUES_FIELD_NAME = "PinotDocID";
+ public static final String LUCENE_INDEX_DOC_ID_COLUMN_NAME =
PINOT_DOC_ID_STORED_FIELD_NAME;
Review Comment:
Non-blocking: Could we remove `LUCENE_INDEX_DOC_ID_COLUMN_NAME` and migrate
its remaining callers to `PINOT_DOC_ID_STORED_FIELD_NAME`? The alias is not
needed for persisted-index compatibility because the field-name string stays
`"DocID"`. Keeping it would only be useful if we intentionally need to preserve
downstream Java source references to the old name.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java:
##########
@@ -57,7 +59,9 @@
/// and realtime from
[org.apache.pinot.segment.local.realtime.impl.invertedindex.RealtimeLuceneTextIndex]
public class LuceneTextIndexCreator extends AbstractTextIndexCreator {
private static final Logger LOGGER =
LoggerFactory.getLogger(LuceneTextIndexCreator.class);
- public static final String LUCENE_INDEX_DOC_ID_COLUMN_NAME = "DocID";
+ public static final String PINOT_DOC_ID_STORED_FIELD_NAME = "DocID";
+ public static final String PINOT_DOC_ID_DOC_VALUES_FIELD_NAME = "PinotDocID";
Review Comment:
Non-blocking: Please document what these two fields contain and when each is
used. Both hold Pinot document IDs, and the alternate numeric use of `"DocID"`
is especially useful to explain. Suggested wording:
```java
/// Stored Pinot document ID used by mutable and legacy immutable indexes.
/// Also the alternate numeric doc-values field when the TEXT column is
named "PinotDocID".
public static final String PINOT_DOC_ID_STORED_FIELD_NAME = "DocID";
/// Preferred numeric doc-values field holding the Pinot document ID in
freshly built immutable indexes.
/// Used to build the Lucene-to-Pinot mapping; omitted when explicitly
configured with Skip.
public static final String PINOT_DOC_ID_DOC_VALUES_FIELD_NAME = "PinotDocID";
```
--
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]
