Skip to content

Commit e393cb4

Browse files
authored
feat: fix literal comparison operator (#275)
- Enable comparison for Fixed type with different length and Decimal type with different scale. - Add some test cases to verify that.
1 parent 2728fc7 commit e393cb4

File tree

3 files changed

+22
-3
lines changed

3 files changed

+22
-3
lines changed

src/iceberg/expression/literal.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ bool Literal::operator==(const Literal& other) const { return (*this <=> other)
348348
// Three-way comparison operator
349349
std::partial_ordering Literal::operator<=>(const Literal& other) const {
350350
// If types are different, comparison is unordered
351-
if (*type_ != *other.type_) {
351+
if (type_->type_id() != other.type_->type_id()) {
352352
return std::partial_ordering::unordered;
353353
}
354354

src/iceberg/expression/literal.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <variant>
2626
#include <vector>
2727

28+
#include "iceberg/iceberg_export.h"
2829
#include "iceberg/result.h"
2930
#include "iceberg/type.h"
3031
#include "iceberg/util/decimal.h"
@@ -129,8 +130,12 @@ class ICEBERG_EXPORT Literal : public util::Formattable {
129130

130131
bool operator==(const Literal& other) const;
131132

132-
/// \brief Compare two PrimitiveLiterals. Both literals must have the same type
133-
/// and should not be AboveMax or BelowMin.
133+
/// \brief Compare two literals of the same primitive type.
134+
/// \param other The other literal to compare with.
135+
/// \return The comparison result as std::partial_ordering. If either side is AboveMax,
136+
/// BelowMin or Null, the result is unordered.
137+
/// Note: This comparison cannot be used for sorting literals if any literal is
138+
/// AboveMax, BelowMin or Null.
134139
std::partial_ordering operator<=>(const Literal& other) const;
135140

136141
/// Check if this literal represents a value above the maximum allowed value

src/iceberg/test/literal_test.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,10 @@ INSTANTIATE_TEST_SUITE_P(
611611
.small_literal = Literal::Decimal(123456, 6, 2),
612612
.large_literal = Literal::Decimal(234567, 6, 2),
613613
.equal_literal = Literal::Decimal(123456, 6, 2)},
614+
ComparisonLiteralTestParam{.test_name = "DecimalDifferentScales",
615+
.small_literal = Literal::Decimal(123456, 6, 2),
616+
.large_literal = Literal::Decimal(1234567, 7, 3),
617+
.equal_literal = Literal::Decimal(1234560, 7, 3)},
614618
ComparisonLiteralTestParam{.test_name = "String",
615619
.small_literal = Literal::String("apple"),
616620
.large_literal = Literal::String("banana"),
@@ -620,11 +624,21 @@ INSTANTIATE_TEST_SUITE_P(
620624
.small_literal = Literal::Binary(std::vector<uint8_t>{0x01, 0x02}),
621625
.large_literal = Literal::Binary(std::vector<uint8_t>{0x01, 0x03}),
622626
.equal_literal = Literal::Binary(std::vector<uint8_t>{0x01, 0x02})},
627+
ComparisonLiteralTestParam{
628+
.test_name = "BinaryDifferentLengths",
629+
.small_literal = Literal::Binary(std::vector<uint8_t>{0x01, 0x02}),
630+
.large_literal = Literal::Binary(std::vector<uint8_t>{0x01, 0x02, 0x03}),
631+
.equal_literal = Literal::Binary(std::vector<uint8_t>{0x01, 0x02})},
623632
ComparisonLiteralTestParam{
624633
.test_name = "Fixed",
625634
.small_literal = Literal::Fixed(std::vector<uint8_t>{0x01, 0x02}),
626635
.large_literal = Literal::Fixed(std::vector<uint8_t>{0x01, 0x03}),
627636
.equal_literal = Literal::Fixed(std::vector<uint8_t>{0x01, 0x02})},
637+
ComparisonLiteralTestParam{
638+
.test_name = "FixedDifferentLengths",
639+
.small_literal = Literal::Fixed(std::vector<uint8_t>{0x01, 0x02}),
640+
.large_literal = Literal::Fixed(std::vector<uint8_t>{0x01, 0x02, 0x03}),
641+
.equal_literal = Literal::Fixed(std::vector<uint8_t>{0x01, 0x02})},
628642
ComparisonLiteralTestParam{.test_name = "Date",
629643
.small_literal = Literal::Date(100),
630644
.large_literal = Literal::Date(200),

0 commit comments

Comments
 (0)