Skip to content

Commit 5b3ce8b

Browse files
AlexFilipImprovingAlexFilipImproving
committed
refactor: simplify Value class vector handling
Signed-off-by: Alexandru Filip <alexandru.filip@improving.com>
1 parent ce8d804 commit 5b3ce8b

3 files changed

Lines changed: 94 additions & 320 deletions

File tree

‎src/expr/value.cc‎

Lines changed: 47 additions & 182 deletions
Original file line numberDiff line numberDiff line change
@@ -307,152 +307,58 @@ Ordering Compare(const Value& l, const Value& r) {
307307
return CompareStrings(l.AsStringView(), r.AsStringView());
308308
}
309309

310-
// Vector operation helper functions
311-
Value ApplyToElements(const Value::Array arr,
312-
std::function<Value(const Value&)> func) {
313-
auto result = std::make_shared<std::vector<Value>>();
314-
result->reserve(arr->size());
315-
316-
for (size_t i = 0; i < arr->size(); ++i) {
317-
Value elem_result = func((*arr)[i]);
318-
result->push_back(std::move(elem_result));
319-
}
320-
321-
return Value(result);
322-
}
323-
324-
Value ApplyWithScalar(const Value::Array arr, const Value& scalar,
325-
std::function<Value(const Value&, const Value&)> func,
326-
bool scalar_on_left) {
327-
auto result = std::make_shared<std::vector<Value>>();
328-
result->reserve(arr->size());
329-
330-
for (size_t i = 0; i < arr->size(); ++i) {
331-
Value elem_result =
332-
scalar_on_left ? func(scalar, (*arr)[i]) : func((*arr)[i], scalar);
333-
result->push_back(std::move(elem_result));
334-
}
335-
336-
return Value(result);
337-
}
338-
339-
Value ApplyElementWise(const Value::Array arr1, const Value::Array arr2,
340-
std::function<Value(const Value&, const Value&)> func) {
341-
if (arr1->size() != arr2->size()) {
342-
std::string error_msg = "Length mismatch: arrays have lengths " +
343-
std::to_string(arr1->size()) + " and " +
344-
std::to_string(arr2->size());
345-
return Value(Value::Nil(error_msg));
346-
}
347-
348-
auto result = std::make_shared<std::vector<Value>>();
349-
result->reserve(arr1->size());
350-
351-
for (size_t i = 0; i < arr1->size(); ++i) {
352-
Value elem_result = func((*arr1)[i], (*arr2)[i]);
353-
result->push_back(std::move(elem_result));
354-
}
355-
356-
return Value(result);
357-
}
358-
359310
Value FuncAdd(const Value& l, const Value& r) {
360-
if (!l.IsArray() && !r.IsArray()) {
361-
auto lv = l.AsDouble();
362-
auto rv = r.AsDouble();
363-
if (lv && rv) {
364-
return Value(lv.value() + rv.value());
365-
} else {
366-
return Value(Value::Nil("Add requires numeric operands"));
367-
}
368-
}
369-
if (l.IsArray() && !r.IsArray()) {
370-
return ApplyWithScalar(l.GetArray(), r, FuncAdd, false);
371-
}
372-
if (!l.IsArray() && r.IsArray()) {
373-
return ApplyWithScalar(r.GetArray(), l, FuncAdd, true);
311+
auto lv = l.AsDouble();
312+
auto rv = r.AsDouble();
313+
if (lv && rv) {
314+
return Value(lv.value() + rv.value());
315+
} else {
316+
return Value(Value::Nil("Could not convert value to a number"));
374317
}
375-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncAdd);
376318
}
377319

378320
Value FuncSub(const Value& l, const Value& r) {
379-
if (!l.IsArray() && !r.IsArray()) {
380-
auto lv = l.AsDouble();
381-
auto rv = r.AsDouble();
382-
if (lv && rv) {
383-
return Value(lv.value() - rv.value());
384-
} else {
385-
return Value(Value::Nil("Subtract requires numeric operands"));
386-
}
387-
}
388-
if (l.IsArray() && !r.IsArray()) {
389-
return ApplyWithScalar(l.GetArray(), r, FuncSub, false);
390-
}
391-
if (!l.IsArray() && r.IsArray()) {
392-
return ApplyWithScalar(r.GetArray(), l, FuncSub, true);
321+
auto lv = l.AsDouble();
322+
auto rv = r.AsDouble();
323+
if (lv && rv) {
324+
return Value(lv.value() - rv.value());
325+
} else {
326+
return Value(Value::Nil("Could not convert value to a number"));
393327
}
394-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncSub);
395328
}
396329

397330
Value FuncMul(const Value& l, const Value& r) {
398-
if (!l.IsArray() && !r.IsArray()) {
399-
auto lv = l.AsDouble();
400-
auto rv = r.AsDouble();
401-
if (lv && rv) {
402-
return Value(lv.value() * rv.value());
403-
} else {
404-
return Value(Value::Nil("Multiply requires numeric operands"));
405-
}
406-
}
407-
if (l.IsArray() && !r.IsArray()) {
408-
return ApplyWithScalar(l.GetArray(), r, FuncMul, false);
409-
}
410-
if (!l.IsArray() && r.IsArray()) {
411-
return ApplyWithScalar(r.GetArray(), l, FuncMul, true);
331+
auto lv = l.AsDouble();
332+
auto rv = r.AsDouble();
333+
if (lv && rv) {
334+
return Value(lv.value() * rv.value());
335+
} else {
336+
return Value(Value::Nil("Could not convert value to a number"));
412337
}
413-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncMul);
414338
}
415339

416340
Value FuncDiv(const Value& l, const Value& r) {
417-
if (!l.IsArray() && !r.IsArray()) {
418-
auto lv = l.AsDouble();
419-
auto rv = r.AsDouble();
420-
if (lv && rv) {
421-
if (rv.value() == 0) {
422-
return Value(std::nan(""));
423-
} else {
424-
return Value(lv.value() / rv.value());
425-
}
341+
auto lv = l.AsDouble();
342+
auto rv = r.AsDouble();
343+
if (lv && rv) {
344+
if (rv.value() == 0) {
345+
return Value(std::nan(""));
426346
} else {
427-
return Value(Value::Nil("Divide requires numeric operands"));
347+
return Value(lv.value() / rv.value());
428348
}
349+
} else {
350+
return Value(Value::Nil("Could not convert value to a number"));
429351
}
430-
if (l.IsArray() && !r.IsArray()) {
431-
return ApplyWithScalar(l.GetArray(), r, FuncDiv, false);
432-
}
433-
if (!l.IsArray() && r.IsArray()) {
434-
return ApplyWithScalar(r.GetArray(), l, FuncDiv, true);
435-
}
436-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncDiv);
437352
}
438353

439354
Value FuncPower(const Value& l, const Value& r) {
440-
if (!l.IsArray() && !r.IsArray()) {
441-
auto lv = l.AsDouble();
442-
auto rv = r.AsDouble();
443-
if (lv && rv) {
444-
return Value(std::pow(lv.value(), rv.value()));
445-
} else {
446-
return Value(Value::Nil("Power requires numeric operands"));
447-
}
448-
}
449-
if (l.IsArray() && !r.IsArray()) {
450-
return ApplyWithScalar(l.GetArray(), r, FuncPower, false);
451-
}
452-
if (!l.IsArray() && r.IsArray()) {
453-
return ApplyWithScalar(r.GetArray(), l, FuncPower, true);
355+
auto lv = l.AsDouble();
356+
auto rv = r.AsDouble();
357+
if (lv && rv) {
358+
return Value(std::pow(lv.value(), rv.value()));
359+
} else {
360+
return Value(Value::Nil("Could not convert value to a number"));
454361
}
455-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncPower);
456362
}
457363

458364
Value FuncLt(const Value& l, const Value& r) { return Value(l < r); }
@@ -492,101 +398,71 @@ Value FuncLand(const Value& l, const Value& r) {
492398
}
493399

494400
Value FuncFloor(const Value& o) {
495-
if (o.IsArray()) {
496-
return ApplyToElements(o.GetArray(), FuncFloor);
497-
}
498401
auto d = o.AsDouble();
499402
if (!d) {
500-
return Value(Value::Nil("floor couldn't convert to a double"));
403+
return Value(std::nan(""));
501404
}
502405
return Value(std::floor(*d));
503406
}
504407

505408
Value FuncCeil(const Value& o) {
506-
if (o.IsArray()) {
507-
return ApplyToElements(o.GetArray(), FuncCeil);
508-
}
509409
auto d = o.AsDouble();
510410
if (!d) {
511-
return Value(Value::Nil("ceil couldn't convert to a double"));
411+
return Value(std::nan(""));
512412
}
513413
return Value(std::ceil(*d));
514414
}
515415

516416
Value FuncAbs(const Value& o) {
517-
if (o.IsArray()) {
518-
return ApplyToElements(o.GetArray(), FuncAbs);
519-
}
520417
auto d = o.AsDouble();
521418
if (!d) {
522-
return Value(Value::Nil("abs couldn't convert to a double"));
419+
return Value(std::nan(""));
523420
}
524421
return Value(std::abs(*d));
525422
}
526423

527424
Value FuncLog(const Value& o) {
528-
if (o.IsArray()) {
529-
return ApplyToElements(o.GetArray(), FuncLog);
530-
}
531425
auto d = o.AsDouble();
532426
if (!d) {
533-
return Value(Value::Nil("log couldn't convert to a double"));
427+
return Value(std::nan(""));
534428
}
535429
return Value(std::log(*d));
536430
}
537431

538432
Value FuncLog2(const Value& o) {
539-
if (o.IsArray()) {
540-
return ApplyToElements(o.GetArray(), FuncLog2);
541-
}
542433
auto d = o.AsDouble();
543434
if (!d) {
544-
return Value(Value::Nil("log2 couldn't convert to a double"));
435+
return Value(std::nan(""));
545436
}
546437
return Value(std::log2(*d));
547438
}
548439

549440
Value FuncExp(const Value& o) {
550-
if (o.IsArray()) {
551-
return ApplyToElements(o.GetArray(), FuncExp);
552-
}
553441
auto d = o.AsDouble();
554442
if (!d) {
555-
return Value(Value::Nil("exp couldn't convert to a double"));
443+
return Value(std::nan(""));
556444
}
557445
return Value(std::exp(*d));
558446
}
559447

560448
Value FuncSqrt(const Value& o) {
561-
if (o.IsArray()) {
562-
return ApplyToElements(o.GetArray(), FuncSqrt);
563-
}
564449
auto d = o.AsDouble();
565450
if (!d) {
566-
return Value(Value::Nil("sqrt couldn't convert to a double"));
451+
return Value(std::nan(""));
567452
}
568453
return Value(std::sqrt(*d));
569454
}
570455

571456
Value FuncStrlen(const Value& o) {
572457
if (o.IsArray()) {
573-
return ApplyToElements(o.GetArray(), FuncStrlen);
458+
return Value();
574459
}
575460
return Value(double(o.AsStringView().size()));
576461
}
577462

578463
Value FuncStartswith(const Value& l, const Value& r) {
579-
bool l_is_vec = l.IsArray();
580-
bool r_is_vec = r.IsArray();
581-
582-
if (l_is_vec && !r_is_vec) {
583-
return ApplyWithScalar(l.GetArray(), r, FuncStartswith, false);
584-
}
585-
if (!l_is_vec && r_is_vec) {
586-
return ApplyWithScalar(r.GetArray(), l, FuncStartswith, true);
587-
}
588-
if (l_is_vec && r_is_vec) {
589-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncStartswith);
464+
if (l.IsArray() || r.IsArray()) {
465+
return Value();
590466
}
591467
auto ls = l.AsStringView();
592468
auto rs = r.AsStringView();
@@ -598,19 +474,8 @@ Value FuncStartswith(const Value& l, const Value& r) {
598474
}
599475

600476
Value FuncContains(const Value& l, const Value& r) {
601-
bool l_is_vec = l.IsArray();
602-
bool r_is_vec = r.IsArray();
603-
604-
if (l_is_vec && !r_is_vec) {
605-
return ApplyWithScalar(l.GetArray(), r, FuncContains, false);
606-
}
607-
608-
if (!l_is_vec && r_is_vec) {
609-
return ApplyWithScalar(r.GetArray(), l, FuncContains, true);
610-
}
611-
612-
if (l_is_vec && r_is_vec) {
613-
return ApplyElementWise(l.GetArray(), r.GetArray(), FuncContains);
477+
if (l.IsArray() || r.IsArray()) {
478+
return Value();
614479
}
615480

616481
auto ls = l.AsStringView();
@@ -630,7 +495,7 @@ Value FuncContains(const Value& l, const Value& r) {
630495

631496
Value FuncSubstr(const Value& l, const Value& m, const Value& r) {
632497
if (l.IsArray() || m.IsArray() || r.IsArray()) {
633-
return Value(Value::Nil("SUBSTR does not accept lists as parameters"));
498+
return Value(Value::Nil("Invalid type for substr. Expected string"));
634499
}
635500

636501
auto ls = l.AsStringView();
@@ -659,7 +524,7 @@ Value FuncSubstr(const Value& l, const Value& m, const Value& r) {
659524

660525
Value FuncLower(const Value& o) {
661526
if (o.IsArray()) {
662-
return ApplyToElements(o.GetArray(), FuncLower);
527+
return Value();
663528
}
664529
auto os = o.AsStringView();
665530
std::string result;
@@ -677,7 +542,7 @@ Value FuncLower(const Value& o) {
677542

678543
Value FuncUpper(const Value& o) {
679544
if (o.IsArray()) {
680-
return ApplyToElements(o.GetArray(), FuncUpper);
545+
return Value();
681546
}
682547
auto os = o.AsStringView();
683548
std::string result;

‎src/expr/value.h‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
#ifndef VALKEYSEARCH_EXPR_VALUE_H
88
#define VALKEYSEARCH_EXPR_VALUE_H
99

10-
#include <functional>
1110
#include <iostream>
1211
#include <memory>
1312
#include <optional>
@@ -131,15 +130,6 @@ static inline std::ostream& operator<<(std::ostream& os, Ordering o) {
131130

132131
Ordering Compare(const Value& l, const Value& r);
133132

134-
// Array operation helper functions
135-
Value ApplyToElements(const Value::Array vec,
136-
std::function<Value(const Value&)> func);
137-
Value ApplyWithScalar(const Value::Array vec, const Value& scalar,
138-
std::function<Value(const Value&, const Value&)> func,
139-
bool scalar_on_left);
140-
Value ApplyElementWise(const Value::Array vec1, const Value::Array vec2,
141-
std::function<Value(const Value&, const Value&)> func);
142-
143133
//
144134
// Comparison operators: kUNORDERED values are not equal to anything.
145135
//

0 commit comments

Comments
 (0)