xiangfu0 commented on code in PR #18092:
URL: https://github.com/apache/pinot/pull/18092#discussion_r3036488959
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java:
##########
@@ -124,6 +130,18 @@ private void
validateForwardIndexEnabled(ForwardIndexConfig forwardIndexConfig,
"Compression codec: %s is not applicable to raw column: %s",
compressionCodec, column);
}
}
+
+ // Validate transformCodec constraints
+ org.apache.pinot.segment.spi.compression.TransformCodec transformCodec =
+ forwardIndexConfig.getTransformCodec();
+ if (transformCodec != null && transformCodec !=
org.apache.pinot.segment.spi.compression.TransformCodec.NONE) {
Review Comment:
No longer applicable — `TransformCodec` was removed. The current code uses
`ChunkCodecPipeline` and `ChunkCodec` which are properly imported.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/writer/impl/BaseChunkForwardIndexWriter.java:
##########
@@ -88,13 +90,40 @@ public abstract class BaseChunkForwardIndexWriter
implements Closeable {
protected BaseChunkForwardIndexWriter(File file, ChunkCompressionType
compressionType, int totalDocs,
int numDocsPerChunk, long chunkSize, int sizeOfEntry, int version,
boolean fixed)
throws IOException {
- Preconditions.checkArgument(version == 2 || version == 3 || (fixed &&
(version == 4 || version == 5)),
+ this(file, compressionType, null, sizeOfEntry, totalDocs, numDocsPerChunk,
chunkSize, sizeOfEntry, version, fixed);
+ }
+
+ /**
+ * Constructor with optional transform codec support.
+ *
+ * @param file Data file to write into
+ * @param compressionType Type of compression
+ * @param transformCodec Optional transform codec to apply before compression
+ * @param valueSizeInBytes Size of each typed value (4 for INT, 8 for LONG);
used by transform
+ * @param totalDocs Total docs to write
+ * @param numDocsPerChunk Number of docs per data chunk
+ * @param chunkSize Size of chunk
+ * @param sizeOfEntry Size of entry (in bytes), max size for variable byte
implementation.
+ * @param version version of File
+ * @param fixed if the data type is fixed width (required for version
validation)
+ * @throws IOException if the file isn't found or can't be mapped
+ */
+ protected BaseChunkForwardIndexWriter(File file, ChunkCompressionType
compressionType,
+ @Nullable TransformCodec transformCodec, int valueSizeInBytes, int
totalDocs,
+ int numDocsPerChunk, long chunkSize, int sizeOfEntry, int version,
boolean fixed)
+ throws IOException {
+ boolean hasTransform = transformCodec != null && transformCodec !=
TransformCodec.NONE;
+ Preconditions.checkArgument(
+ version == 2 || version == 3 || (fixed && (version == 4 || version ==
5 || version == 6)),
"Illegal version: %s for %s bytes values", version, fixed ? "fixed" :
"variable");
+ if (hasTransform) {
+ Preconditions.checkArgument(version == 6, "TransformCodec requires
writer version 6, got: %s", version);
+ }
Preconditions.checkArgument(chunkSize <= Integer.MAX_VALUE, "Chunk size
limited to 2GB");
_chunkSize = (int) chunkSize;
- _chunkCompressor = ChunkCompressorFactory.getCompressor(compressionType);
+ _chunkCompressor = ChunkCompressorFactory.getCompressor(compressionType,
transformCodec, valueSizeInBytes);
Review Comment:
Already addressed — the constructor now validates:
`Preconditions.checkArgument(valueSizeInBytes == Integer.BYTES ||
valueSizeInBytes == Long.BYTES, ...)` when the pipeline has transforms.
--
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]
