Skip to content

Commit 3e10fe3

Browse files
Add query changes to support unsigned-long in star tree (#17275)
Signed-off-by: Shailesh Singh <shaikumm@amazon.com>
1 parent 9bbdd3c commit 3e10fe3

File tree

10 files changed

+263
-47
lines changed

10 files changed

+263
-47
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3535
- Add SearchService and Search GRPC endpoint ([#17830](https://github.com/opensearch-project/OpenSearch/pull/17830))
3636
- Add update and delete support in pull-based ingestion ([#17822](https://github.com/opensearch-project/OpenSearch/pull/17822))
3737
- Allow maxPollSize and pollTimeout in IngestionSource to be configurable ([#17863](https://github.com/opensearch-project/OpenSearch/pull/17863))
38+
- [Star Tree] [Search] Add query changes to support unsigned-long in star tree ([#17275](https://github.com/opensearch-project/OpenSearch/pull/17275))
3839

3940
### Changed
4041
- Migrate BC libs to their FIPS counterparts ([#14912](https://github.com/opensearch-project/OpenSearch/pull/14912))

server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionDataType.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@
1212

1313
/**
1414
* Represents the data type of the dimension value.
15+
* TODO: This needs to be eventually merged with DimensionFilterMapper and all indexing related code
16+
* which use this should instead use the mapper
1517
*
1618
* @opensearch.experimental
1719
*/
1820
@ExperimentalApi
1921
public enum DimensionDataType {
2022
LONG {
2123
@Override
22-
int compare(Long a, Long b) {
24+
public int compare(Long a, Long b) {
2325
if (a == null && b == null) {
2426
return 0;
2527
}
@@ -34,7 +36,7 @@ int compare(Long a, Long b) {
3436
},
3537
UNSIGNED_LONG {
3638
@Override
37-
int compare(Long a, Long b) {
39+
public int compare(Long a, Long b) {
3840
if (a == null && b == null) {
3941
return 0;
4042
}
@@ -48,5 +50,5 @@ int compare(Long a, Long b) {
4850
}
4951
};
5052

51-
abstract int compare(Long a, Long b);
53+
public abstract int compare(Long a, Long b);
5254
}

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,15 @@
88
package org.opensearch.index.compositeindex.datacube.startree.fileformats.node;
99

1010
import org.apache.lucene.store.RandomAccessInput;
11+
import org.opensearch.index.compositeindex.datacube.DimensionDataType;
1112
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNode;
1213
import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNodeType;
1314
import org.opensearch.search.startree.StarTreeNodeCollector;
15+
import org.opensearch.search.startree.filter.provider.DimensionFilterMapper;
1416

1517
import java.io.IOException;
1618
import java.io.UncheckedIOException;
19+
import java.util.Comparator;
1720
import java.util.Iterator;
1821

1922
/**
@@ -193,15 +196,23 @@ public StarTreeNode getChildStarNode() throws IOException {
193196
}
194197

195198
@Override
196-
public StarTreeNode getChildForDimensionValue(Long dimensionValue, StarTreeNode lastMatchedChild) throws IOException {
199+
public StarTreeNode getChildForDimensionValue(
200+
Long dimensionValue,
201+
StarTreeNode lastMatchedChild,
202+
DimensionFilterMapper dimensionFilterMapper
203+
) throws IOException {
197204
// there will be no children for leaf nodes
198205
if (isLeaf()) {
199206
return null;
200207
}
201208

202209
StarTreeNode resultStarTreeNode = null;
203210
if (null != dimensionValue) {
204-
resultStarTreeNode = binarySearchChild(dimensionValue, lastMatchedChild);
211+
resultStarTreeNode = binarySearchChild(
212+
dimensionValue,
213+
lastMatchedChild,
214+
dimensionFilterMapper == null ? DimensionDataType.LONG::compare : dimensionFilterMapper.comparator()
215+
);
205216
}
206217
return resultStarTreeNode;
207218
}
@@ -238,11 +249,13 @@ private static FixedLengthStarTreeNode matchStarTreeNodeTypeOrNull(FixedLengthSt
238249
* Performs a binary search to find a child node with the given dimension value.
239250
*
240251
* @param dimensionValue The dimension value to search for
252+
* @param lastMatchedNode : If not null, we begin the binary search from the node after this.
253+
* @param comparator : Comparator (LONG or UNSIGNED_LONG) to compare the dimension values
241254
* @return The child node if found, null otherwise
242255
* @throws IOException If there's an error reading from the input
243256
*/
244-
private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, StarTreeNode lastMatchedNode) throws IOException {
245-
257+
private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, StarTreeNode lastMatchedNode, Comparator<Long> comparator)
258+
throws IOException {
246259
int low = firstChildId;
247260

248261
int high = getInt(LAST_CHILD_ID_OFFSET);
@@ -268,10 +281,10 @@ private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, StarTreeN
268281
int mid = low + (high - low) / 2;
269282
FixedLengthStarTreeNode midNode = new FixedLengthStarTreeNode(in, mid);
270283
long midDimensionValue = midNode.getDimensionValue();
271-
272-
if (midDimensionValue == dimensionValue) {
284+
int compare = comparator.compare(midDimensionValue, dimensionValue);
285+
if (compare == 0) {
273286
return midNode;
274-
} else if (midDimensionValue < dimensionValue) {
287+
} else if (compare < 0) {
275288
low = mid + 1;
276289
} else {
277290
high = mid - 1;
@@ -281,16 +294,19 @@ private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, StarTreeN
281294
}
282295

283296
@Override
284-
public void collectChildrenInRange(long low, long high, StarTreeNodeCollector collector) throws IOException {
285-
if (low <= high) {
286-
FixedLengthStarTreeNode lowStarTreeNode = binarySearchChild(low, true, null);
297+
public void collectChildrenInRange(long low, long high, StarTreeNodeCollector collector, DimensionFilterMapper dimensionFilterMapper)
298+
throws IOException {
299+
Comparator<Long> comparator = dimensionFilterMapper.comparator();
300+
if (comparator.compare(low, high) <= 0) {
301+
FixedLengthStarTreeNode lowStarTreeNode = binarySearchChild(low, true, null, comparator);
287302
if (lowStarTreeNode != null) {
288-
FixedLengthStarTreeNode highStarTreeNode = binarySearchChild(high, false, lowStarTreeNode);
303+
FixedLengthStarTreeNode highStarTreeNode = binarySearchChild(high, false, lowStarTreeNode, comparator);
289304
if (highStarTreeNode != null) {
290305
for (int lowNodeId = lowStarTreeNode.nodeId(); lowNodeId <= highStarTreeNode.nodeId(); ++lowNodeId) {
291306
collector.collectStarTreeNode(new FixedLengthStarTreeNode(in, lowNodeId));
292307
}
293-
} else if (lowStarTreeNode.getDimensionValue() <= high) { // Low StarTreeNode is the last default node for that dimension.
308+
} else if (comparator.compare(lowStarTreeNode.getDimensionValue(), high) <= 0) { // Low StarTreeNode is the last default//
309+
// node for that dimension.
294310
collector.collectStarTreeNode(lowStarTreeNode);
295311
}
296312
}
@@ -302,11 +318,16 @@ public void collectChildrenInRange(long low, long high, StarTreeNodeCollector co
302318
* @param dimensionValue : The dimension to match.
303319
* @param matchNextHighest : If true then we try to return @dimensionValue or the next Highest. Else, we return @dimensionValue or the next Lowest.
304320
* @param lastMatchedNode : If not null, we begin the binary search from the node after this.
321+
* @param comparator : Comparator (LONG or UNSIGNED_LONG) to compare the dimension values
305322
* @return : Matched node or null.
306323
* @throws IOException :
307324
*/
308-
private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, boolean matchNextHighest, StarTreeNode lastMatchedNode)
309-
throws IOException {
325+
private FixedLengthStarTreeNode binarySearchChild(
326+
long dimensionValue,
327+
boolean matchNextHighest,
328+
StarTreeNode lastMatchedNode,
329+
Comparator<Long> comparator
330+
) throws IOException {
310331

311332
int low = firstChildId;
312333
int tempLow = low;
@@ -342,17 +363,18 @@ private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, boolean m
342363
FixedLengthStarTreeNode midNode = new FixedLengthStarTreeNode(in, mid);
343364
long midDimensionValue = midNode.getDimensionValue();
344365

345-
if (midDimensionValue == dimensionValue) {
366+
int compare = comparator.compare(midDimensionValue, dimensionValue);
367+
if (compare == 0) {
346368
return midNode;
347369
} else {
348-
if (midDimensionValue < dimensionValue) { // Going to the right from mid to search next
370+
if (compare < 0) { // Going to the right from mid to search next
349371
tempLow = mid + 1;
350372
// We are going out of bounds for this dimension on the right side.
351373
if (tempLow > high || tempLow == nullNodeId) {
352374
return matchNextHighest ? null : midNode;
353375
} else {
354376
FixedLengthStarTreeNode nodeGreaterThanMid = new FixedLengthStarTreeNode(in, tempLow);
355-
if (nodeGreaterThanMid.getDimensionValue() > dimensionValue) {
377+
if (comparator.compare(nodeGreaterThanMid.getDimensionValue(), dimensionValue) > 0) {
356378
return matchNextHighest ? nodeGreaterThanMid : midNode;
357379
}
358380
}
@@ -363,7 +385,7 @@ private FixedLengthStarTreeNode binarySearchChild(long dimensionValue, boolean m
363385
return matchNextHighest ? midNode : null;
364386
} else {
365387
FixedLengthStarTreeNode nodeLessThanMid = new FixedLengthStarTreeNode(in, tempHigh);
366-
if (nodeLessThanMid.getDimensionValue() < dimensionValue) {
388+
if (comparator.compare(nodeLessThanMid.getDimensionValue(), dimensionValue) < 0) {
367389
return matchNextHighest ? midNode : nodeLessThanMid;
368390
}
369391
}

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/node/StarTreeNode.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import org.opensearch.common.annotation.ExperimentalApi;
1212
import org.opensearch.search.startree.StarTreeNodeCollector;
13+
import org.opensearch.search.startree.filter.provider.DimensionFilterMapper;
1314

1415
import java.io.IOException;
1516
import java.util.Iterator;
@@ -107,28 +108,34 @@ public interface StarTreeNode {
107108
* @param dimensionValue the dimension value
108109
* @return the child node for the given dimension value or null if child is not present
109110
* @throws IOException if an I/O error occurs while retrieving the child node
111+
*
112+
* TODO: Remove this method - Only used in UTs for Star Tree indexing
110113
*/
111114
default StarTreeNode getChildForDimensionValue(Long dimensionValue) throws IOException {
112-
return getChildForDimensionValue(dimensionValue, null);
115+
return getChildForDimensionValue(dimensionValue, null, null);
113116
}
114117

115118
/**
116119
* Matches the given @dimensionValue amongst the child default nodes for this node.
117120
* @param dimensionValue : Value to match
118121
* @param lastMatchedChild : If not null, binary search will use this as the start/low
122+
* @param dimensionFilterMapper : dimensionFilterMapper object
119123
* @return : Matched StarTreeNode or null if not found
120124
* @throws IOException : Any exception in reading the node data from index.
121125
*/
122-
StarTreeNode getChildForDimensionValue(Long dimensionValue, StarTreeNode lastMatchedChild) throws IOException;
126+
StarTreeNode getChildForDimensionValue(Long dimensionValue, StarTreeNode lastMatchedChild, DimensionFilterMapper dimensionFilterMapper)
127+
throws IOException;
123128

124129
/**
125130
* Collects all matching child nodes whose dimension values lie within the range of low and high, both inclusive.
126131
* @param low : Starting of the range ( inclusive )
127132
* @param high : End of the range ( inclusive )
128133
* @param collector : Collector to collect the matched child StarTreeNode's
134+
* @param dimensionFilterMapper : dimensionFilterMapper object
129135
* @throws IOException : Any exception in reading the node data from index.
130136
*/
131-
void collectChildrenInRange(long low, long high, StarTreeNodeCollector collector) throws IOException;
137+
void collectChildrenInRange(long low, long high, StarTreeNodeCollector collector, DimensionFilterMapper dimensionFilterMapper)
138+
throws IOException;
132139

133140
/**
134141
* Returns the child star node for a node in the star-tree.

server/src/main/java/org/opensearch/search/startree/filter/ExactMatchDimFilter.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ public class ExactMatchDimFilter implements DimensionFilter {
3535
// Order is essential for successive binary search
3636
private TreeSet<Long> convertedOrdinals;
3737

38+
private DimensionFilterMapper dimensionFilterMapper;
39+
3840
public ExactMatchDimFilter(String dimensionName, List<Object> valuesToMatch) {
3941
this.dimensionName = dimensionName;
4042
this.rawValues = valuesToMatch;
@@ -47,9 +49,10 @@ public void initialiseForSegment(StarTreeValues starTreeValues, SearchContext se
4749
dimensionName,
4850
starTreeValues.getStarTreeField().getDimensionsOrder()
4951
);
50-
DimensionFilterMapper dimensionFilterMapper = DimensionFilterMapper.Factory.fromMappedFieldType(
52+
this.dimensionFilterMapper = DimensionFilterMapper.Factory.fromMappedFieldType(
5153
searchContext.mapperService().fieldType(dimensionName)
5254
);
55+
5356
for (Object rawValue : rawValues) {
5457
Optional<Long> ordinal = dimensionFilterMapper.getMatchingOrdinal(
5558
matchedDim.getField(),
@@ -69,7 +72,7 @@ public void matchStarTreeNodes(StarTreeNode parentNode, StarTreeValues starTreeV
6972
if (parentNode != null) {
7073
StarTreeNode lastMatchedNode = null;
7174
for (long ordinal : convertedOrdinals) {
72-
lastMatchedNode = parentNode.getChildForDimensionValue(ordinal, lastMatchedNode);
75+
lastMatchedNode = parentNode.getChildForDimensionValue(ordinal, lastMatchedNode, dimensionFilterMapper);
7376
if (lastMatchedNode != null) {
7477
collector.collectStarTreeNode(lastMatchedNode);
7578
}

server/src/main/java/org/opensearch/search/startree/filter/RangeMatchDimFilter.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public class RangeMatchDimFilter implements DimensionFilter {
3737

3838
private boolean skipRangeCollection = false;
3939

40+
private DimensionFilterMapper dimensionFilterMapper;
41+
4042
public RangeMatchDimFilter(String dimensionName, Object low, Object high, boolean includeLow, boolean includeHigh) {
4143
this.dimensionName = dimensionName;
4244
this.low = low;
@@ -48,9 +50,10 @@ public RangeMatchDimFilter(String dimensionName, Object low, Object high, boolea
4850
@Override
4951
public void initialiseForSegment(StarTreeValues starTreeValues, SearchContext searchContext) {
5052
skipRangeCollection = false;
51-
DimensionFilterMapper dimensionFilterMapper = DimensionFilterMapper.Factory.fromMappedFieldType(
53+
this.dimensionFilterMapper = DimensionFilterMapper.Factory.fromMappedFieldType(
5254
searchContext.mapperService().fieldType(dimensionName)
5355
);
56+
5457
lowOrdinal = 0L;
5558
if (low != null) {
5659
MatchType lowMatchType = includeLow ? MatchType.GTE : MatchType.GT;
@@ -77,13 +80,14 @@ public void initialiseForSegment(StarTreeValues starTreeValues, SearchContext se
7780
public void matchStarTreeNodes(StarTreeNode parentNode, StarTreeValues starTreeValues, StarTreeNodeCollector collector)
7881
throws IOException {
7982
if (parentNode != null && !skipRangeCollection) {
80-
parentNode.collectChildrenInRange(lowOrdinal, highOrdinal, collector);
83+
parentNode.collectChildrenInRange(lowOrdinal, highOrdinal, collector, dimensionFilterMapper);
8184
}
8285
}
8386

8487
@Override
8588
public boolean matchDimValue(long ordinal, StarTreeValues starTreeValues) {
86-
return lowOrdinal <= ordinal && ordinal <= highOrdinal;
89+
return dimensionFilterMapper.comparator().compare(lowOrdinal, ordinal) <= 0
90+
&& dimensionFilterMapper.comparator().compare(ordinal, highOrdinal) <= 0;
8791
}
8892

8993
}

server/src/main/java/org/opensearch/search/startree/filter/provider/DimensionFilterMapper.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
import org.apache.lucene.sandbox.document.HalfFloatPoint;
1515
import org.apache.lucene.util.BytesRef;
1616
import org.apache.lucene.util.NumericUtils;
17+
import org.opensearch.common.Numbers;
1718
import org.opensearch.common.annotation.ExperimentalApi;
1819
import org.opensearch.common.lucene.BytesRefs;
1920
import org.opensearch.common.lucene.Lucene;
21+
import org.opensearch.index.compositeindex.datacube.DimensionDataType;
2022
import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
2123
import org.opensearch.index.compositeindex.datacube.startree.utils.iterator.SortedSetStarTreeValuesIterator;
2224
import org.opensearch.index.mapper.KeywordFieldMapper.KeywordFieldType;
@@ -29,6 +31,7 @@
2931

3032
import java.io.IOException;
3133
import java.util.ArrayList;
34+
import java.util.Comparator;
3235
import java.util.List;
3336
import java.util.Map;
3437
import java.util.Optional;
@@ -40,6 +43,7 @@
4043
import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.INTEGER;
4144
import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.LONG;
4245
import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.SHORT;
46+
import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.UNSIGNED_LONG;
4347
import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.hasDecimalPart;
4448
import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.signum;
4549

@@ -88,6 +92,10 @@ Optional<Long> getMatchingOrdinal(
8892
DimensionFilter.MatchType matchType
8993
);
9094

95+
default Comparator<Long> comparator() {
96+
return DimensionDataType.LONG::compare;
97+
}
98+
9199
/**
92100
* Singleton Factory for @{@link DimensionFilterMapper}
93101
*/
@@ -109,7 +117,9 @@ class Factory {
109117
DOUBLE.typeName(),
110118
new DoubleFieldMapperNumeric(),
111119
org.opensearch.index.mapper.KeywordFieldMapper.CONTENT_TYPE,
112-
new KeywordFieldMapper()
120+
new KeywordFieldMapper(),
121+
UNSIGNED_LONG.typeName(),
122+
new UnsignedLongFieldMapperNumeric()
113123
);
114124

115125
public static DimensionFilterMapper fromMappedFieldType(MappedFieldType mappedFieldType) {
@@ -208,6 +218,25 @@ Long defaultMaximum() {
208218
}
209219
}
210220

221+
class UnsignedLongFieldMapperNumeric extends NumericNonDecimalMapper {
222+
223+
@Override
224+
Long defaultMinimum() {
225+
return Numbers.MIN_UNSIGNED_LONG_VALUE_AS_LONG;
226+
}
227+
228+
@Override
229+
Long defaultMaximum() {
230+
return Numbers.MAX_UNSIGNED_LONG_VALUE_AS_LONG;
231+
}
232+
233+
@Override
234+
public Comparator<Long> comparator() {
235+
return DimensionDataType.UNSIGNED_LONG::compare;
236+
}
237+
238+
}
239+
211240
abstract class NumericDecimalFieldMapper extends NumericMapper {
212241

213242
@Override

0 commit comments

Comments
 (0)