Skip to content

Commit 2ac7f0a

Browse files
author
Yao Liu
committed
address comments
1 parent 0826d0c commit 2ac7f0a

File tree

8 files changed

+24
-41
lines changed

8 files changed

+24
-41
lines changed

‎pinot-core/src/main/java/org/apache/pinot/core/common/DataBlockCache.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,6 @@ public int[] getNumValuesForMVColumn(String column) {
511511
return numValues;
512512
}
513513

514-
515514
private boolean markLoaded(FieldSpec.DataType dataType, String column) {
516515
return _columnValueLoaded.computeIfAbsent(dataType, k -> new HashSet<>()).add(column);
517516
}

‎pinot-core/src/main/java/org/apache/pinot/core/common/DataFetcher.java‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -622,15 +622,8 @@ void readBytesValues(int[] docIds, int length, byte[][] valueBuffer) {
622622
_reader.readDictIds(docIds, length, dictIdBuffer, readerContext);
623623
_dictionary.readBytesValues(dictIdBuffer, length, valueBuffer);
624624
} else {
625-
switch (_storedType) {
626-
case BYTES:
627-
case STRING:
628-
for (int i = 0; i < length; i++) {
629-
valueBuffer[i] = _reader.getBytes(docIds[i], readerContext);
630-
}
631-
break;
632-
default:
633-
throw new IllegalStateException();
625+
for (int i = 0; i < length; i++) {
626+
valueBuffer[i] = _reader.getBytes(docIds[i], readerContext);
634627
}
635628
}
636629
}
@@ -771,7 +764,6 @@ public void readNumValuesMV(int[] docIds, int length, int[] numValuesBuffer) {
771764
}
772765
}
773766

774-
775767
private int[] getSVDictIdsBuffer() {
776768
return _dictionary == null ? null : THREAD_LOCAL_DICT_IDS.get();
777769
}

‎pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunction.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -608,13 +608,7 @@ public byte[][][] transformToBytesValuesMV(ProjectionBlock projectionBlock) {
608608
} else {
609609
assert getResultMetadata().getDataType().getStoredType() == DataType.STRING;
610610
String[][] stringValuesMV = transformToStringValuesMV(projectionBlock);
611-
for (int i = 0; i < length; i++) {
612-
String[] stringValues = stringValuesMV[i];
613-
int numValues = stringValues.length;
614-
byte[][] bytesValues = new byte[numValues][];
615-
ArrayCopyUtils.copy(stringValues, bytesValues, numValues);
616-
_bytesValuesMV[i] = bytesValues;
617-
}
611+
ArrayCopyUtils.copy(stringValuesMV, _bytesValuesMV, length);
618612
}
619613
return _bytesValuesMV;
620614
}

‎pinot-core/src/test/java/org/apache/pinot/core/common/DataFetcherTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ public void testFetchStringValues() {
250250
public void testFetchBytesValues() {
251251
testFetchBytesValues(BYTES_COLUMN);
252252
testFetchBytesValues(NO_DICT_BYTES_COLUMN);
253+
testFetchBytesValues(STRING_COLUMN);
254+
testFetchBytesValues(NO_DICT_STRING_COLUMN);
253255
}
254256

255257
@Test
@@ -361,8 +363,7 @@ public void testFetchBytesValues(String column) {
361363
_dataFetcher.fetchBytesValues(column, docIds, length, bytesValues);
362364

363365
for (int i = 0; i < length; i++) {
364-
String stringValue = Integer.toString(_values[docIds[i]]);
365-
Assert.assertEquals(new String(bytesValues[i], UTF_8), stringValue, ERROR_MESSAGE);
366+
Assert.assertEquals(new String(bytesValues[i], UTF_8), Integer.toString(_values[docIds[i]]), ERROR_MESSAGE);
366367
}
367368
}
368369

‎pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/OnHeapStringDictionary.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
2626
import org.apache.pinot.spi.data.FieldSpec.DataType;
2727

28+
import static java.nio.charset.StandardCharsets.UTF_8;
29+
2830

2931
/**
3032
* Implementation of String dictionary that cache all values on-heap.
@@ -55,10 +57,9 @@ public OnHeapStringDictionary(PinotDataBuffer dataBuffer, int length, int numByt
5557
_unPaddedStringToIdMap.defaultReturnValue(Dictionary.NULL_VALUE_INDEX);
5658

5759
for (int i = 0; i < length; i++) {
58-
String unpaddedString = getUnpaddedString(i, buffer);
59-
_unpaddedStrings[i] = unpaddedString;
60-
_unPaddedStringToIdMap.put(unpaddedString, i);
6160
_unpaddedBytes[i] = getUnpaddedBytes(i, buffer);
61+
_unpaddedStrings[i] = new String(_unpaddedBytes[i], UTF_8);
62+
_unPaddedStringToIdMap.put(_unpaddedStrings[i], i);
6263
}
6364

6465
if (paddingByte == 0) {

‎pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readers/ImmutableDictionaryTypeConversionTest.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
2222
import java.io.File;
2323
import java.math.BigDecimal;
24-
import java.nio.charset.StandardCharsets;
2524
import java.util.Arrays;
2625
import java.util.Random;
2726
import org.apache.commons.io.FileUtils;
@@ -40,6 +39,7 @@
4039
import org.testng.annotations.BeforeClass;
4140
import org.testng.annotations.Test;
4241

42+
import static java.nio.charset.StandardCharsets.UTF_8;
4343
import static org.testng.Assert.assertEquals;
4444

4545

@@ -60,13 +60,6 @@ public class ImmutableDictionaryTypeConversionTest {
6060
private static final int STRING_LENGTH = 6;
6161
private static final int BYTES_LENGTH = STRING_LENGTH / 2;
6262

63-
private static byte[] toUTF8Bytes(String stringValue) {
64-
return stringValue.getBytes(StandardCharsets.UTF_8);
65-
}
66-
private static ByteArray toUTF8ByeArray(String stringValue) {
67-
return new ByteArray(toUTF8Bytes(stringValue));
68-
}
69-
7063
private int[] _intValues;
7164
private long[] _longValues;
7265
private float[] _floatValues;
@@ -120,7 +113,7 @@ public void setUp()
120113
_utf8BytesValues = new ByteArray[NUM_VALUES];
121114
for (int i = 0; i < NUM_VALUES; i++) {
122115
_bytesValues[i] = BytesUtils.toByteArray(_stringValues[i]);
123-
_utf8BytesValues[i] = toUTF8ByeArray(_stringValues[i]);
116+
_utf8BytesValues[i] = new ByteArray(_stringValues[i].getBytes(UTF_8));
124117
}
125118

126119
try (SegmentDictionaryCreator dictionaryCreator = new SegmentDictionaryCreator(

‎pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/ForwardIndexReader.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -732,15 +732,8 @@ default void readValuesMV(int[] docIds, int length, int maxNumValuesPerMVEntry,
732732
* @param context Reader context
733733
*/
734734
default void readValuesMV(int[] docIds, int length, int maxNumValuesPerMVEntry, byte[][][] values, T context) {
735-
switch (getStoredType()) {
736-
case STRING:
737-
case BYTES:
738-
for (int i = 0; i < length; i++) {
739-
values[i] = getBytesMV(docIds[i], context);
740-
}
741-
break;
742-
default:
743-
throw new IllegalArgumentException("readValuesMV not supported for type " + getStoredType());
735+
for (int i = 0; i < length; i++) {
736+
values[i] = getBytesMV(docIds[i], context);
744737
}
745738
}
746739

‎pinot-spi/src/main/java/org/apache/pinot/spi/utils/ArrayCopyUtils.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,16 @@ public static void copy(String[] src, byte[][] dest, int length) {
278278
}
279279
}
280280

281+
public static void copy(String[][] src, byte[][][] dest, int length) {
282+
for (int i = 0; i < length; i++) {
283+
String[] stringValues = src[i];
284+
int numValues = stringValues.length;
285+
byte[][] bytesValues = new byte[numValues][];
286+
ArrayCopyUtils.copy(stringValues, bytesValues, numValues);
287+
dest[i] = bytesValues;
288+
}
289+
}
290+
281291
public static void copy(byte[][] src, BigDecimal[] dest, int length) {
282292
for (int i = 0; i < length; i++) {
283293
dest[i] = BigDecimalUtils.deserialize(src[i]);

0 commit comments

Comments
 (0)