siddharthteotia commented on a change in pull request #6719:
URL: https://github.com/apache/incubator-pinot/pull/6719#discussion_r620638226
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java
##########
@@ -235,33 +234,32 @@ public void init(SegmentGeneratorConfig
segmentCreationSpec, SegmentIndexCreatio
Preconditions.checkState(!invertedIndexColumns.contains(columnName),
"Cannot create inverted index for raw index column: %s",
columnName);
- ChunkCompressionType compressionType =
- getColumnCompressionType(segmentCreationSpec, fieldSpec);
+ ChunkCompressionType compressionType =
getColumnCompressionType(segmentCreationSpec, fieldSpec);
// Initialize forward index creator
boolean deriveNumDocsPerChunk =
shouldDeriveNumDocsPerChunk(columnName,
segmentCreationSpec.getColumnProperties());
int writerVersion = rawIndexWriterVersion(columnName,
segmentCreationSpec.getColumnProperties());
_forwardIndexCreatorMap.put(columnName,
- getRawIndexCreatorForColumn(_indexDir, compressionType,
columnName, fieldSpec.getDataType(), totalDocs,
+ getRawIndexCreatorForColumn(_indexDir, compressionType,
columnName, storedType, totalDocs,
indexCreationInfo.getLengthOfLongestEntry(),
deriveNumDocsPerChunk, writerVersion));
}
if (textIndexColumns.contains(columnName)) {
// Initialize text index creator
Preconditions.checkState(fieldSpec.isSingleValueField(),
"Text index is currently only supported on single-value columns");
- Preconditions.checkState(fieldSpec.getDataType() == STRING,
- "Text index is currently only supported on STRING type columns");
+ Preconditions
+ .checkState(storedType == DataType.STRING, "Text index is
currently only supported on STRING type columns");
Review comment:
With the current ongoing discussion for nested/json types, flattening
etc, the short term will have JSON data type and the forward index will
continue to have it as STRING as stored type.
The above check will actually pass for a json column if the user wants to
create a text index on it. I don't think we want that at least until we have
tested how text index will work on json (stringified ) data. I think the check
should still happen using the logical type and not the stored type
cc @amrishlal
--
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.
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
