Skip to content

Commit 67276c0

Browse files
gibber9809gibber9809
authored
fix(clp-s): Simplify timestamp range index evaluation code; Fix conversion utility used to compare float AST literals to integer values (fixes #1375). (#1369)
1 parent 5fab7df commit 67276c0

9 files changed

Lines changed: 192 additions & 212 deletions

‎components/core/src/clp_s/TimestampEntry.cpp‎

Lines changed: 98 additions & 205 deletions
Original file line numberDiff line numberDiff line change
@@ -147,107 +147,57 @@ EvaluatedValue TimestampEntry::evaluate_filter(FilterOperation op, double timest
147147
return EvaluatedValue::Unknown;
148148
}
149149

150-
if (m_encoding == DoubleEpoch) {
151-
switch (op) {
152-
case FilterOperation::EQ:
153-
if (timestamp >= m_epoch_start_double && timestamp <= m_epoch_end_double) {
154-
return EvaluatedValue::Unknown;
155-
} else {
156-
return EvaluatedValue::False;
157-
}
158-
case FilterOperation::NEQ:
159-
if (timestamp >= m_epoch_start_double && timestamp <= m_epoch_end_double) {
160-
return EvaluatedValue::Unknown;
161-
} else {
162-
return EvaluatedValue::True;
163-
}
164-
case FilterOperation::LT:
165-
if (timestamp > m_epoch_end_double) {
166-
return EvaluatedValue::True;
167-
} else if (timestamp <= m_epoch_start_double) {
168-
return EvaluatedValue::False;
169-
} else {
170-
return EvaluatedValue::Unknown;
171-
}
172-
case FilterOperation::LTE:
173-
if (timestamp >= m_epoch_end_double) {
174-
return EvaluatedValue::True;
175-
} else if (timestamp < m_epoch_start_double) {
176-
return EvaluatedValue::False;
177-
} else {
178-
return EvaluatedValue::Unknown;
179-
}
180-
case FilterOperation::GT:
181-
if (timestamp < m_epoch_start_double) {
182-
return EvaluatedValue::True;
183-
} else if (timestamp >= m_epoch_end_double) {
184-
return EvaluatedValue::False;
185-
} else {
186-
return EvaluatedValue::Unknown;
187-
}
188-
case FilterOperation::GTE:
189-
if (timestamp <= m_epoch_start_double) {
190-
return EvaluatedValue::True;
191-
} else if (timestamp > m_epoch_end_double) {
192-
return EvaluatedValue::False;
193-
} else {
194-
return EvaluatedValue::Unknown;
195-
}
196-
default:
150+
if (DoubleEpoch != m_encoding) {
151+
return EvaluatedValue::Unknown;
152+
}
153+
154+
switch (op) {
155+
case FilterOperation::EQ:
156+
if (timestamp >= m_epoch_start_double && timestamp <= m_epoch_end_double) {
197157
return EvaluatedValue::Unknown;
198-
}
199-
} else if (m_encoding == Epoch) {
200-
double epoch_start_tmp = m_epoch_start, epoch_end_tmp = m_epoch_end;
201-
switch (op) {
202-
case FilterOperation::EQ:
203-
if (timestamp >= epoch_start_tmp && timestamp <= epoch_end_tmp) {
204-
return EvaluatedValue::Unknown;
205-
} else {
206-
return EvaluatedValue::False;
207-
}
208-
case FilterOperation::NEQ:
209-
if (timestamp >= epoch_start_tmp && timestamp <= epoch_end_tmp) {
210-
return EvaluatedValue::Unknown;
211-
} else {
212-
return EvaluatedValue::True;
213-
}
214-
case FilterOperation::LT:
215-
if (timestamp > epoch_end_tmp) {
216-
return EvaluatedValue::True;
217-
} else if (timestamp <= epoch_start_tmp) {
218-
return EvaluatedValue::False;
219-
} else {
220-
return EvaluatedValue::Unknown;
221-
}
222-
case FilterOperation::LTE:
223-
if (timestamp >= epoch_end_tmp) {
224-
return EvaluatedValue::True;
225-
} else if (timestamp < epoch_start_tmp) {
226-
return EvaluatedValue::False;
227-
} else {
228-
return EvaluatedValue::Unknown;
229-
}
230-
case FilterOperation::GT:
231-
if (timestamp < epoch_start_tmp) {
232-
return EvaluatedValue::True;
233-
} else if (timestamp >= epoch_end_tmp) {
234-
return EvaluatedValue::False;
235-
} else {
236-
return EvaluatedValue::Unknown;
237-
}
238-
case FilterOperation::GTE:
239-
if (timestamp <= epoch_start_tmp) {
240-
return EvaluatedValue::True;
241-
} else if (timestamp > epoch_end_tmp) {
242-
return EvaluatedValue::False;
243-
} else {
244-
return EvaluatedValue::Unknown;
245-
}
246-
default:
158+
} else {
159+
return EvaluatedValue::False;
160+
}
161+
case FilterOperation::NEQ:
162+
if (timestamp >= m_epoch_start_double && timestamp <= m_epoch_end_double) {
247163
return EvaluatedValue::Unknown;
248-
}
249-
} else {
250-
return EvaluatedValue::Unknown;
164+
} else {
165+
return EvaluatedValue::True;
166+
}
167+
case FilterOperation::LT:
168+
if (timestamp > m_epoch_end_double) {
169+
return EvaluatedValue::True;
170+
} else if (timestamp <= m_epoch_start_double) {
171+
return EvaluatedValue::False;
172+
} else {
173+
return EvaluatedValue::Unknown;
174+
}
175+
case FilterOperation::LTE:
176+
if (timestamp >= m_epoch_end_double) {
177+
return EvaluatedValue::True;
178+
} else if (timestamp < m_epoch_start_double) {
179+
return EvaluatedValue::False;
180+
} else {
181+
return EvaluatedValue::Unknown;
182+
}
183+
case FilterOperation::GT:
184+
if (timestamp < m_epoch_start_double) {
185+
return EvaluatedValue::True;
186+
} else if (timestamp >= m_epoch_end_double) {
187+
return EvaluatedValue::False;
188+
} else {
189+
return EvaluatedValue::Unknown;
190+
}
191+
case FilterOperation::GTE:
192+
if (timestamp <= m_epoch_start_double) {
193+
return EvaluatedValue::True;
194+
} else if (timestamp > m_epoch_end_double) {
195+
return EvaluatedValue::False;
196+
} else {
197+
return EvaluatedValue::Unknown;
198+
}
199+
default:
200+
return EvaluatedValue::Unknown;
251201
}
252202
}
253203

@@ -256,114 +206,57 @@ EvaluatedValue TimestampEntry::evaluate_filter(FilterOperation op, epochtime_t t
256206
return EvaluatedValue::Unknown;
257207
}
258208

259-
if (m_encoding == DoubleEpoch) {
260-
/**
261-
* TODO: this borrows logic from the double_as_int function
262-
* should
263-
*/
264-
epochtime_t epoch_start_tmp_ltgte = std::ceil(m_epoch_start_double);
265-
epochtime_t epoch_start_tmp_gtlte = std::floor(m_epoch_start_double);
266-
epochtime_t epoch_end_tmp_ltgte = std::ceil(m_epoch_end_double);
267-
epochtime_t epoch_end_tmp_gtlte = std::floor(m_epoch_end_double);
268-
switch (op) {
269-
case FilterOperation::EQ:
270-
if (timestamp >= epoch_start_tmp_ltgte && timestamp <= epoch_end_tmp_gtlte) {
271-
return EvaluatedValue::Unknown;
272-
} else {
273-
return EvaluatedValue::False;
274-
}
275-
case FilterOperation::NEQ:
276-
if (timestamp >= epoch_start_tmp_ltgte && timestamp <= epoch_end_tmp_gtlte) {
277-
return EvaluatedValue::Unknown;
278-
} else {
279-
return EvaluatedValue::True;
280-
}
281-
case FilterOperation::LT:
282-
if (timestamp > epoch_end_tmp_gtlte) {
283-
return EvaluatedValue::True;
284-
} else if (timestamp <= epoch_start_tmp_gtlte) {
285-
return EvaluatedValue::False;
286-
} else {
287-
return EvaluatedValue::Unknown;
288-
}
289-
case FilterOperation::LTE:
290-
if (timestamp >= epoch_end_tmp_ltgte) {
291-
return EvaluatedValue::True;
292-
} else if (timestamp < epoch_start_tmp_ltgte) {
293-
return EvaluatedValue::False;
294-
} else {
295-
return EvaluatedValue::Unknown;
296-
}
297-
case FilterOperation::GT:
298-
if (timestamp < epoch_start_tmp_ltgte) {
299-
return EvaluatedValue::True;
300-
} else if (timestamp >= epoch_end_tmp_ltgte) {
301-
return EvaluatedValue::False;
302-
} else {
303-
return EvaluatedValue::Unknown;
304-
}
305-
case FilterOperation::GTE:
306-
if (timestamp <= epoch_start_tmp_gtlte) {
307-
return EvaluatedValue::True;
308-
} else if (timestamp > epoch_end_tmp_gtlte) {
309-
return EvaluatedValue::False;
310-
} else {
311-
return EvaluatedValue::Unknown;
312-
}
313-
default:
209+
if (Epoch != m_encoding) {
210+
return EvaluatedValue::Unknown;
211+
}
212+
213+
switch (op) {
214+
case FilterOperation::EQ:
215+
if (timestamp >= m_epoch_start && timestamp <= m_epoch_end) {
314216
return EvaluatedValue::Unknown;
315-
}
316-
} else if (m_encoding == Epoch) {
317-
switch (op) {
318-
case FilterOperation::EQ:
319-
if (timestamp >= m_epoch_start && timestamp <= m_epoch_end) {
320-
return EvaluatedValue::Unknown;
321-
} else {
322-
return EvaluatedValue::False;
323-
}
324-
case FilterOperation::NEQ:
325-
if (timestamp >= m_epoch_start && timestamp <= m_epoch_end) {
326-
return EvaluatedValue::Unknown;
327-
} else {
328-
return EvaluatedValue::True;
329-
}
330-
case FilterOperation::LT:
331-
if (timestamp > m_epoch_end) {
332-
return EvaluatedValue::True;
333-
} else if (timestamp <= m_epoch_start) {
334-
return EvaluatedValue::False;
335-
} else {
336-
return EvaluatedValue::Unknown;
337-
}
338-
case FilterOperation::LTE:
339-
if (timestamp >= m_epoch_end) {
340-
return EvaluatedValue::True;
341-
} else if (timestamp < m_epoch_start) {
342-
return EvaluatedValue::False;
343-
} else {
344-
return EvaluatedValue::Unknown;
345-
}
346-
case FilterOperation::GT:
347-
if (timestamp < m_epoch_start) {
348-
return EvaluatedValue::True;
349-
} else if (timestamp >= m_epoch_end) {
350-
return EvaluatedValue::False;
351-
} else {
352-
return EvaluatedValue::Unknown;
353-
}
354-
case FilterOperation::GTE:
355-
if (timestamp <= m_epoch_start) {
356-
return EvaluatedValue::True;
357-
} else if (timestamp > m_epoch_end) {
358-
return EvaluatedValue::False;
359-
} else {
360-
return EvaluatedValue::Unknown;
361-
}
362-
default:
217+
} else {
218+
return EvaluatedValue::False;
219+
}
220+
case FilterOperation::NEQ:
221+
if (timestamp >= m_epoch_start && timestamp <= m_epoch_end) {
363222
return EvaluatedValue::Unknown;
364-
}
365-
} else {
366-
return EvaluatedValue::Unknown;
223+
} else {
224+
return EvaluatedValue::True;
225+
}
226+
case FilterOperation::LT:
227+
if (timestamp > m_epoch_end) {
228+
return EvaluatedValue::True;
229+
} else if (timestamp <= m_epoch_start) {
230+
return EvaluatedValue::False;
231+
} else {
232+
return EvaluatedValue::Unknown;
233+
}
234+
case FilterOperation::LTE:
235+
if (timestamp >= m_epoch_end) {
236+
return EvaluatedValue::True;
237+
} else if (timestamp < m_epoch_start) {
238+
return EvaluatedValue::False;
239+
} else {
240+
return EvaluatedValue::Unknown;
241+
}
242+
case FilterOperation::GT:
243+
if (timestamp < m_epoch_start) {
244+
return EvaluatedValue::True;
245+
} else if (timestamp >= m_epoch_end) {
246+
return EvaluatedValue::False;
247+
} else {
248+
return EvaluatedValue::Unknown;
249+
}
250+
case FilterOperation::GTE:
251+
if (timestamp <= m_epoch_start) {
252+
return EvaluatedValue::True;
253+
} else if (timestamp > m_epoch_end) {
254+
return EvaluatedValue::False;
255+
} else {
256+
return EvaluatedValue::Unknown;
257+
}
258+
default:
259+
return EvaluatedValue::Unknown;
367260
}
368261
}
369262

‎components/core/src/clp_s/TimestampEntry.hpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ class TimestampEntry {
118118
*/
119119
epochtime_t get_end_timestamp() const;
120120

121+
auto get_timestamp_encoding() const -> TimestampEncoding { return m_encoding; }
122+
121123
private:
122124
TimestampEncoding m_encoding;
123125
double m_epoch_start_double, m_epoch_end_double;

‎components/core/src/clp_s/search/EvaluateTimestampIndex.cpp‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,23 @@ EvaluatedValue EvaluateTimestampIndex::run(std::shared_ptr<Expression> const& ex
9696
auto const filter_op{filter->get_operation()};
9797
int64_t int_literal{};
9898
double float_literal{};
99-
if (literal->as_int(int_literal, filter_op)) {
100-
ret = range_it->second->evaluate_filter(filter_op, int_literal);
101-
} else if (literal->as_float(float_literal, filter_op)) {
102-
ret = range_it->second->evaluate_filter(filter_op, float_literal);
103-
} else {
104-
return EvaluatedValue::Unknown;
99+
100+
auto const encoding{range_it->second->get_timestamp_encoding()};
101+
switch (encoding) {
102+
case TimestampEntry::TimestampEncoding::DoubleEpoch:
103+
if (false == literal->as_float(float_literal, filter_op)) {
104+
return EvaluatedValue::Unknown;
105+
}
106+
ret = range_it->second->evaluate_filter(filter_op, float_literal);
107+
break;
108+
case TimestampEntry::TimestampEncoding::Epoch:
109+
if (false == literal->as_int(int_literal, filter_op)) {
110+
return EvaluatedValue::Unknown;
111+
}
112+
ret = range_it->second->evaluate_filter(filter_op, int_literal);
113+
break;
114+
default:
115+
return EvaluatedValue::Unknown;
105116
}
106117

107118
if (ret == EvaluatedValue::True) {

‎components/core/src/clp_s/search/ast/SearchUtils.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,14 @@ bool double_as_int(double in, FilterOperation op, int64_t& out) {
5252
case FilterOperation::LT:
5353
case FilterOperation::GTE:
5454
out = std::ceil(in);
55+
break;
5556
case FilterOperation::GT:
5657
case FilterOperation::LTE:
5758
out = std::floor(in);
59+
break;
5860
default:
5961
out = static_cast<int64_t>(in);
62+
break;
6063
}
6164
return true;
6265
}

‎components/core/tests/clp_s_test_utils.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../src/clp_s/ArchiveWriter.hpp"
1111
#include "../src/clp_s/InputConfig.hpp"
1212
#include "../src/clp_s/JsonParser.hpp"
13+
#include "../src/clp_s/TimestampPattern.hpp"
1314

1415
auto compress_archive(
1516
std::string const& file_path,
@@ -45,6 +46,7 @@ auto compress_archive(
4546
parser_option.timestamp_key = std::move(timestamp_key.value());
4647
}
4748

49+
clp_s::TimestampPattern::init();
4850
clp_s::JsonParser parser{parser_option};
4951
std::vector<clp_s::ArchiveStats> archive_stats;
5052
REQUIRE(parser.ingest());

0 commit comments

Comments
 (0)