Skip to content

Commit 83308bf

Browse files
fulmicoton-ddfulmicoton-dd
committed
fixing unit test
1 parent 01bc7f2 commit 83308bf

File tree

4 files changed

+91
-94
lines changed

4 files changed

+91
-94
lines changed

‎src/collector/sort_key/mod.rs‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod sort_key_computer;
33
use columnar::StrColumn;
44
pub use sort_key_computer::{SegmentSortKeyComputer, SortKeyComputer};
55

6+
use crate::fastfield::FastValue;
67
use crate::termdict::TermOrdinal;
78
use crate::{DocId, Order, Score};
89

@@ -36,12 +37,8 @@ impl<TSegmentSortKeyComputer, TSegmentSortKey> SegmentSortKeyComputer
3637
for (TSegmentSortKeyComputer, Order)
3738
where
3839
TSegmentSortKeyComputer: SegmentSortKeyComputer<SegmentSortKey = TSegmentSortKey>,
39-
TSegmentSortKey: ReverseOrder<ReverseOrderType = TSegmentSortKey>
40-
+ PartialOrd
41-
+ Clone
42-
+ 'static
43-
+ Sync
44-
+ Send,
40+
TSegmentSortKey:
41+
ReverseOrder<ReverseType = TSegmentSortKey> + PartialOrd + Clone + 'static + Sync + Send,
4542
{
4643
type SortKey = TSegmentSortKeyComputer::SortKey;
4744
type SegmentSortKey = TSegmentSortKey;
@@ -70,53 +67,55 @@ where
7067
//
7168
// We then rely on an ReverseOrder implementation with a ReverseOrderType that maps to Self.
7269
pub trait ReverseOrder: Clone {
73-
type ReverseOrderType: PartialOrd + Clone;
70+
type ReverseType: PartialOrd + Clone;
7471

75-
fn to_reverse_type(self) -> Self::ReverseOrderType;
72+
fn to_reverse_type(self) -> Self::ReverseType;
7673

77-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self;
74+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self;
7875
}
7976

8077
fn reverse_if_asc<T>(value: T, order: Order) -> T
81-
where T: ReverseOrder<ReverseOrderType = T> {
78+
where T: ReverseOrder<ReverseType = T> {
8279
match order {
8380
Order::Asc => value.to_reverse_type(),
8481
Order::Desc => value,
8582
}
8683
}
8784

88-
impl ReverseOrder for u64 {
89-
type ReverseOrderType = u64;
85+
impl<TFastValue: FastValue> ReverseOrder for TFastValue {
86+
type ReverseType = TFastValue;
9087

91-
fn to_reverse_type(self) -> Self::ReverseOrderType {
92-
u64::MAX - self
88+
fn to_reverse_type(self) -> Self::ReverseType {
89+
// TODO check that the compiler is good enough to compile that to i64::MAX - self for i64
90+
// for instance.
91+
TFastValue::from_u64(u64::MAX - self.to_u64())
9392
}
9493

95-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
94+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
9695
reverse_value.to_reverse_type()
9796
}
9897
}
9998

10099
impl ReverseOrder for u32 {
101-
type ReverseOrderType = u32;
100+
type ReverseType = u32;
102101

103-
fn to_reverse_type(self) -> Self::ReverseOrderType {
102+
fn to_reverse_type(self) -> Self::ReverseType {
104103
u32::MAX - self
105104
}
106105

107-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
106+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
108107
reverse_value.to_reverse_type()
109108
}
110109
}
111110

112111
impl ReverseOrder for f32 {
113-
type ReverseOrderType = f32;
112+
type ReverseType = f32;
114113

115-
fn to_reverse_type(self) -> Self::ReverseOrderType {
114+
fn to_reverse_type(self) -> Self::ReverseType {
116115
f32::MAX - self
117116
}
118117

119-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
118+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
120119
// That's an involution
121120
reverse_value.to_reverse_type()
122121
}
@@ -125,13 +124,13 @@ impl ReverseOrder for f32 {
125124
// The point here is that for Option, we do not want None values to come on top
126125
// when running a Asc query.
127126
impl<T: ReverseOrder> ReverseOrder for Option<T> {
128-
type ReverseOrderType = Option<T::ReverseOrderType>;
127+
type ReverseType = Option<T::ReverseType>;
129128

130-
fn to_reverse_type(self) -> Self::ReverseOrderType {
129+
fn to_reverse_type(self) -> Self::ReverseType {
131130
self.map(|val| val.to_reverse_type())
132131
}
133132

134-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
133+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
135134
reverse_value.map(T::from_reverse_type)
136135
}
137136
}

‎src/collector/sort_key/sort_key_computer.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,37 +107,37 @@ where
107107
}
108108

109109
impl<T: PartialOrd + Clone> ReverseOrder for std::cmp::Reverse<T> {
110-
type ReverseOrderType = T;
110+
type ReverseType = T;
111111

112-
fn to_reverse_type(self) -> Self::ReverseOrderType {
112+
fn to_reverse_type(self) -> Self::ReverseType {
113113
self.0
114114
}
115115

116-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
116+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
117117
Self(reverse_value)
118118
}
119119
}
120120

121121
impl ReverseOrder for String {
122-
type ReverseOrderType = std::cmp::Reverse<String>;
122+
type ReverseType = std::cmp::Reverse<String>;
123123

124-
fn to_reverse_type(self) -> Self::ReverseOrderType {
124+
fn to_reverse_type(self) -> Self::ReverseType {
125125
std::cmp::Reverse(self)
126126
}
127127

128-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
128+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
129129
reverse_value.0
130130
}
131131
}
132132

133133
impl<Left: ReverseOrder, Right: ReverseOrder> ReverseOrder for (Left, Right) {
134-
type ReverseOrderType = (Left::ReverseOrderType, Right::ReverseOrderType);
134+
type ReverseType = (Left::ReverseType, Right::ReverseType);
135135

136-
fn to_reverse_type(self) -> Self::ReverseOrderType {
136+
fn to_reverse_type(self) -> Self::ReverseType {
137137
(self.0.to_reverse_type(), self.1.to_reverse_type())
138138
}
139139

140-
fn from_reverse_type(reverse_value: Self::ReverseOrderType) -> Self {
140+
fn from_reverse_type(reverse_value: Self::ReverseType) -> Self {
141141
(
142142
Left::from_reverse_type(reverse_value.0),
143143
Right::from_reverse_type(reverse_value.1),

‎src/collector/sort_key_top_collector.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::marker::PhantomData;
2+
13
use crate::collector::sort_key::{ReverseOrder, SegmentSortKeyComputer, SortKeyComputer};
24
use crate::collector::top_collector::{merge_fruits, TopCollector, TopSegmentCollector};
35
use crate::collector::{Collector, SegmentCollector};
@@ -56,7 +58,7 @@ where
5658
Order::Asc => {
5759
let reverse_segment_fruits: Vec<
5860
Vec<(
59-
<TSortKeyComputer::SortKey as ReverseOrder>::ReverseOrderType,
61+
<TSortKeyComputer::SortKey as ReverseOrder>::ReverseType,
6062
DocAddress,
6163
)>,
6264
> = segment_fruits

‎src/collector/top_score_collector.rs‎

Lines changed: 56 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use std::fmt;
22
use std::marker::PhantomData;
3-
use std::sync::Arc;
43

5-
use columnar::ColumnValues;
4+
use columnar::Column;
65
use serde::{Deserialize, Serialize};
76

87
use super::Collector;
@@ -137,43 +136,34 @@ impl fmt::Debug for TopDocs {
137136
}
138137
}
139138

140-
struct SortKeyByFastFieldReader {
141-
sort_column: Arc<dyn ColumnValues<u64>>,
142-
order: Order,
139+
struct SortKeyByFastFieldReader<T> {
140+
sort_column: Column<u64>,
141+
typ: PhantomData<T>,
143142
}
144143

145-
impl SegmentSortKeyComputer for SortKeyByFastFieldReader {
146-
type SortKey = u64;
144+
impl<T: FastValue> SegmentSortKeyComputer for SortKeyByFastFieldReader<T> {
145+
type SortKey = Option<T>;
147146

148-
type SegmentSortKey = u64;
147+
type SegmentSortKey = Option<u64>;
149148

150149
fn sort_key(&mut self, doc: DocId, _score: Score) -> Self::SegmentSortKey {
151-
let val = self.sort_column.get_val(doc);
152-
if self.order == Order::Desc {
153-
u64::MAX - val
154-
} else {
155-
val
156-
}
150+
self.sort_column.first(doc)
157151
}
158152

159153
fn convert_segment_sort_key(&self, sort_key: Self::SegmentSortKey) -> Self::SortKey {
160-
if self.order == Order::Desc {
161-
u64::MAX - sort_key
162-
} else {
163-
sort_key
164-
}
154+
sort_key.map(T::from_u64)
165155
}
166156
}
167157

168-
struct ScorerByField {
158+
struct ScorerByField<T: FastValue = u64> {
169159
field: String,
170-
order: Order,
160+
typ: PhantomData<T>,
171161
}
172162

173-
impl SortKeyComputer for ScorerByField {
174-
type Child = SortKeyByFastFieldReader;
163+
impl<T: FastValue> SortKeyComputer for ScorerByField<T> {
164+
type Child = SortKeyByFastFieldReader<T>;
175165

176-
type SortKey = u64;
166+
type SortKey = Option<T>;
177167

178168
fn segment_sort_key_computer(
179169
&self,
@@ -189,13 +179,9 @@ impl SortKeyComputer for ScorerByField {
189179
sort_column_opt.ok_or_else(|| FastFieldNotAvailableError {
190180
field_name: self.field.clone(),
191181
})?;
192-
let mut default_value = 0u64;
193-
if self.order.is_asc() {
194-
default_value = u64::MAX;
195-
}
196182
Ok(SortKeyByFastFieldReader {
197-
sort_column: sort_column.first_or_default_col(default_value),
198-
order: self.order.clone(),
183+
sort_column: sort_column,
184+
typ: PhantomData,
199185
})
200186
}
201187
}
@@ -329,12 +315,15 @@ impl TopDocs {
329315
self,
330316
field: impl ToString,
331317
order: Order,
332-
) -> impl Collector<Fruit = Vec<(u64, DocAddress)>> {
318+
) -> impl Collector<Fruit = Vec<(Option<u64>, DocAddress)>> {
333319
TopBySortKeyCollector::new(
334-
ScorerByField {
335-
field: field.to_string(),
320+
(
321+
ScorerByField {
322+
field: field.to_string(),
323+
typ: PhantomData,
324+
},
336325
order,
337-
},
326+
),
338327
self.0.into_different_sort_key_type(),
339328
)
340329
}
@@ -412,16 +401,20 @@ impl TopDocs {
412401
self,
413402
fast_field: impl ToString,
414403
order: Order,
415-
) -> impl Collector<Fruit = Vec<(TFastValue, DocAddress)>>
404+
) -> impl Collector<Fruit = Vec<(Option<TFastValue>, DocAddress)>>
416405
where
417-
TFastValue: FastValue,
406+
TFastValue: FastValue + ReverseOrder,
418407
{
419-
let u64_collector = self.order_by_u64_field(fast_field.to_string(), order);
420-
FastFieldConvertCollector {
421-
collector: u64_collector,
422-
field: fast_field.to_string(),
423-
fast_value: PhantomData,
424-
}
408+
TopBySortKeyCollector::new(
409+
(
410+
ScorerByField {
411+
field: fast_field.to_string(),
412+
typ: PhantomData,
413+
},
414+
order,
415+
),
416+
self.0.into_different_sort_key_type(),
417+
)
425418
}
426419

427420
/// Like `order_by_fast_field`, but for a `String` fast field.
@@ -1378,13 +1371,13 @@ mod tests {
13781371
let searcher = index.reader()?.searcher();
13791372

13801373
let top_collector = TopDocs::with_limit(4).order_by_u64_field(SIZE, Order::Desc);
1381-
let top_docs: Vec<(u64, DocAddress)> = searcher.search(&query, &top_collector)?;
1374+
let top_docs: Vec<(Option<u64>, DocAddress)> = searcher.search(&query, &top_collector)?;
13821375
assert_eq!(
13831376
&top_docs[..],
13841377
&[
1385-
(64, DocAddress::new(0, 1)),
1386-
(16, DocAddress::new(0, 2)),
1387-
(12, DocAddress::new(0, 0))
1378+
(Some(64), DocAddress::new(0, 1)),
1379+
(Some(16), DocAddress::new(0, 2)),
1380+
(Some(12), DocAddress::new(0, 0))
13881381
]
13891382
);
13901383
Ok(())
@@ -1417,12 +1410,13 @@ mod tests {
14171410
index_writer.commit()?;
14181411
let searcher = index.reader()?.searcher();
14191412
let top_collector = TopDocs::with_limit(3).order_by_fast_field("birthday", Order::Desc);
1420-
let top_docs: Vec<(DateTime, DocAddress)> = searcher.search(&AllQuery, &top_collector)?;
1413+
let top_docs: Vec<(Option<DateTime>, DocAddress)> =
1414+
searcher.search(&AllQuery, &top_collector)?;
14211415
assert_eq!(
14221416
&top_docs[..],
14231417
&[
1424-
(mr_birthday, DocAddress::new(0, 1)),
1425-
(pr_birthday, DocAddress::new(0, 0)),
1418+
(Some(mr_birthday), DocAddress::new(0, 1)),
1419+
(Some(pr_birthday), DocAddress::new(0, 0)),
14261420
]
14271421
);
14281422
Ok(())
@@ -1447,12 +1441,13 @@ mod tests {
14471441
index_writer.commit()?;
14481442
let searcher = index.reader()?.searcher();
14491443
let top_collector = TopDocs::with_limit(3).order_by_fast_field("altitude", Order::Desc);
1450-
let top_docs: Vec<(i64, DocAddress)> = searcher.search(&AllQuery, &top_collector)?;
1444+
let top_docs: Vec<(Option<i64>, DocAddress)> =
1445+
searcher.search(&AllQuery, &top_collector)?;
14511446
assert_eq!(
14521447
&top_docs[..],
14531448
&[
1454-
(40i64, DocAddress::new(0, 1)),
1455-
(-1i64, DocAddress::new(0, 0)),
1449+
(Some(40i64), DocAddress::new(0, 1)),
1450+
(Some(-1i64), DocAddress::new(0, 0)),
14561451
]
14571452
);
14581453
Ok(())
@@ -1477,12 +1472,13 @@ mod tests {
14771472
index_writer.commit()?;
14781473
let searcher = index.reader()?.searcher();
14791474
let top_collector = TopDocs::with_limit(3).order_by_fast_field("altitude", Order::Desc);
1480-
let top_docs: Vec<(f64, DocAddress)> = searcher.search(&AllQuery, &top_collector)?;
1475+
let top_docs: Vec<(Option<f64>, DocAddress)> =
1476+
searcher.search(&AllQuery, &top_collector)?;
14811477
assert_eq!(
14821478
&top_docs[..],
14831479
&[
1484-
(40f64, DocAddress::new(0, 1)),
1485-
(-1.0f64, DocAddress::new(0, 0)),
1480+
(Some(40f64), DocAddress::new(0, 1)),
1481+
(Some(-1.0f64), DocAddress::new(0, 0)),
14861482
]
14871483
);
14881484
Ok(())
@@ -1789,14 +1785,14 @@ mod tests {
17891785
let searcher = index.reader()?.searcher();
17901786

17911787
let top_collector = TopDocs::with_limit(4).order_by_fast_field(SIZE, Order::Asc);
1792-
let top_docs: Vec<(u64, DocAddress)> = searcher.search(&query, &top_collector)?;
1788+
let top_docs: Vec<(Option<u64>, DocAddress)> = searcher.search(&query, &top_collector)?;
17931789
assert_eq!(
17941790
&top_docs[..],
17951791
&[
1796-
(12, DocAddress::new(0, 0)),
1797-
(16, DocAddress::new(0, 2)),
1798-
(64, DocAddress::new(0, 1)),
1799-
(18446744073709551615, DocAddress::new(0, 3)),
1792+
(Some(12), DocAddress::new(0, 0)),
1793+
(Some(16), DocAddress::new(0, 2)),
1794+
(Some(64), DocAddress::new(0, 1)),
1795+
(None, DocAddress::new(0, 3)),
18001796
]
18011797
);
18021798
Ok(())

0 commit comments

Comments
 (0)