Skip to content

Commit 252b2ab

Browse files
zhjwpkuzhjwpku
committed
fix: use std::unique_ptr
1 parent 4ff9280 commit 252b2ab

File tree

4 files changed

+36
-29
lines changed

4 files changed

+36
-29
lines changed

‎src/iceberg/test/transform_test.cc‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,8 @@ TEST_F(TransformProjectTest, IdentityProjectEquality) {
964964
EXPECT_EQ(projected->op(), Expression::Operation::kEq);
965965

966966
auto unbound_projected =
967-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected);
967+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
968+
std::move(projected));
968969
ASSERT_NE(unbound_projected, nullptr);
969970
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kEq);
970971
EXPECT_EQ(unbound_projected->literals().size(), 1);
@@ -1028,7 +1029,8 @@ TEST_F(TransformProjectTest, IdentityProjectSet) {
10281029
ASSERT_NE(projected_in, nullptr);
10291030
EXPECT_EQ(projected_in->op(), Expression::Operation::kIn);
10301031
auto unbound_projected =
1031-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected_in);
1032+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1033+
std::move(projected_in));
10321034
ASSERT_NE(unbound_projected, nullptr);
10331035
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kIn);
10341036
EXPECT_EQ(unbound_projected->literals().size(), 3);
@@ -1054,7 +1056,8 @@ TEST_F(TransformProjectTest, BucketProjectEquality) {
10541056
EXPECT_EQ(projected->op(), Expression::Operation::kEq);
10551057

10561058
auto unbound_projected =
1057-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected);
1059+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1060+
std::move(projected));
10581061
ASSERT_NE(unbound_projected, nullptr);
10591062
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kEq);
10601063
EXPECT_EQ(unbound_projected->literals().size(), 1);
@@ -1084,7 +1087,8 @@ TEST_F(TransformProjectTest, BucketProjectWithMatchingTransformedChild) {
10841087
ASSERT_NE(projected, nullptr);
10851088
EXPECT_EQ(projected->op(), Expression::Operation::kEq);
10861089
auto unbound_projected =
1087-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected);
1090+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1091+
std::move(projected));
10881092
ASSERT_NE(unbound_projected, nullptr);
10891093
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kEq);
10901094
EXPECT_EQ(unbound_projected->literals().size(), 1);
@@ -1152,7 +1156,8 @@ TEST_F(TransformProjectTest, TruncateProjectIntEquality) {
11521156
EXPECT_EQ(projected->op(), Expression::Operation::kEq);
11531157

11541158
auto unbound_projected =
1155-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected);
1159+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1160+
std::move(projected));
11561161
ASSERT_NE(unbound_projected, nullptr);
11571162
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kEq);
11581163
EXPECT_EQ(unbound_projected->literals().size(), 1);
@@ -1189,7 +1194,8 @@ TEST_F(TransformProjectTest, TruncateProjectIntGreaterThan) {
11891194
EXPECT_EQ(projected->op(), Expression::Operation::kGtEq);
11901195

11911196
auto unbound_projected =
1192-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected);
1197+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1198+
std::move(projected));
11931199
ASSERT_NE(unbound_projected, nullptr);
11941200
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kGtEq);
11951201
EXPECT_EQ(unbound_projected->literals().size(), 1);
@@ -1210,7 +1216,8 @@ TEST_F(TransformProjectTest, TruncateProjectStringEquality) {
12101216
EXPECT_EQ(projected->op(), Expression::Operation::kEq);
12111217

12121218
auto unbound_projected =
1213-
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(projected);
1219+
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1220+
std::move(projected));
12141221
ASSERT_NE(unbound_projected, nullptr);
12151222
EXPECT_EQ(unbound_projected->op(), Expression::Operation::kEq);
12161223
EXPECT_EQ(unbound_projected->literals().size(), 1);
@@ -1235,7 +1242,7 @@ TEST_F(TransformProjectTest, TruncateProjectStringStartsWith) {
12351242

12361243
auto unbound_projected_short =
12371244
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1238-
projected_short);
1245+
std::move(projected_short));
12391246
ASSERT_NE(unbound_projected_short, nullptr);
12401247
EXPECT_EQ(unbound_projected_short->op(), Expression::Operation::kStartsWith);
12411248
EXPECT_EQ(unbound_projected_short->literals().size(), 1);
@@ -1256,7 +1263,7 @@ TEST_F(TransformProjectTest, TruncateProjectStringStartsWith) {
12561263

12571264
auto unbound_projected_equal =
12581265
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
1259-
projected_equal);
1266+
std::move(projected_equal));
12601267
ASSERT_NE(unbound_projected_equal, nullptr);
12611268
EXPECT_EQ(unbound_projected_equal->op(), Expression::Operation::kEq);
12621269
EXPECT_EQ(unbound_projected_equal->literals().size(), 1);

‎src/iceberg/transform.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ bool Transform::SatisfiesOrderOf(const Transform& other) const {
246246
std::unreachable();
247247
}
248248

249-
Result<std::shared_ptr<UnboundPredicate>> Transform::Project(
249+
Result<std::unique_ptr<UnboundPredicate>> Transform::Project(
250250
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate) {
251251
switch (transform_type_) {
252252
case TransformType::kIdentity:

‎src/iceberg/transform.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ class ICEBERG_EXPORT Transform : public util::Formattable {
177177
/// Projected(transform(value)) is true.
178178
/// \param name The name of the partition column.
179179
/// \param predicate The predicate to project.
180-
/// \return A Result containing either a shared pointer to the projected predicate or an
180+
/// \return A Result containing either a unique pointer to the projected predicate or an
181181
/// Error if the projection fails.
182-
Result<std::shared_ptr<UnboundPredicate>> Project(
182+
Result<std::unique_ptr<UnboundPredicate>> Project(
183183
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate);
184184

185185
/// \brief Returns a string representation of this transform (e.g., "bucket[16]").

‎src/iceberg/util/projection_util_internal.h‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ namespace iceberg {
3939

4040
class ProjectionUtil {
4141
private:
42-
static Result<std::shared_ptr<UnboundPredicate>> TransformSet(
42+
static Result<std::unique_ptr<UnboundPredicate>> TransformSet(
4343
std::string_view name, const std::shared_ptr<BoundSetPredicate>& predicate,
4444
const std::shared_ptr<TransformFunction>& func) {
4545
std::vector<Literal> transformed;
@@ -57,7 +57,7 @@ class ProjectionUtil {
5757

5858
// General transform for all literal predicates. This is used as a fallback for special
5959
// cases that are not handled by the other transform functions.
60-
static Result<std::shared_ptr<UnboundPredicate>> GenericTransform(
60+
static Result<std::unique_ptr<UnboundPredicate>> GenericTransform(
6161
std::unique_ptr<NamedReference> ref,
6262
const std::shared_ptr<BoundLiteralPredicate>& predicate,
6363
const std::shared_ptr<TransformFunction>& func) {
@@ -88,7 +88,7 @@ class ProjectionUtil {
8888
}
8989
}
9090

91-
static Result<std::shared_ptr<UnboundPredicate>> TruncateByteArray(
91+
static Result<std::unique_ptr<UnboundPredicate>> TruncateByteArray(
9292
std::string_view name, const std::shared_ptr<BoundLiteralPredicate>& predicate,
9393
const std::shared_ptr<TransformFunction>& func) {
9494
ICEBERG_ASSIGN_OR_RAISE(auto ref, NamedReference::Make(std::string(name)));
@@ -107,7 +107,7 @@ class ProjectionUtil {
107107

108108
template <typename T>
109109
requires std::is_same_v<T, int32_t> || std::is_same_v<T, int64_t>
110-
static Result<std::shared_ptr<UnboundPredicate>> TruncateInteger(
110+
static Result<std::unique_ptr<UnboundPredicate>> TruncateInteger(
111111
std::string_view name, const std::shared_ptr<BoundLiteralPredicate>& predicate,
112112
const std::shared_ptr<TransformFunction>& func) {
113113
const Literal& literal = predicate->literal();
@@ -159,7 +159,7 @@ class ProjectionUtil {
159159
}
160160
}
161161

162-
static Result<std::shared_ptr<UnboundPredicate>> TransformTemporal(
162+
static Result<std::unique_ptr<UnboundPredicate>> TransformTemporal(
163163
std::string_view name, const std::shared_ptr<BoundLiteralPredicate>& predicate,
164164
const std::shared_ptr<TransformFunction>& func) {
165165
const Literal& literal = predicate->literal();
@@ -251,7 +251,7 @@ class ProjectionUtil {
251251
}
252252
}
253253

254-
static Result<std::shared_ptr<UnboundPredicate>> TruncateDecimal(
254+
static Result<std::unique_ptr<UnboundPredicate>> TruncateDecimal(
255255
std::string_view name, const std::shared_ptr<BoundLiteralPredicate>& predicate,
256256
const std::shared_ptr<TransformFunction>& func) {
257257
const Literal& boundary = predicate->literal();
@@ -288,7 +288,7 @@ class ProjectionUtil {
288288
}
289289
}
290290

291-
static Result<std::shared_ptr<UnboundPredicate>> TruncateStringLiteral(
291+
static Result<std::unique_ptr<UnboundPredicate>> TruncateStringLiteral(
292292
std::string_view name, const std::shared_ptr<BoundLiteralPredicate>& predicate,
293293
const std::shared_ptr<TransformFunction>& func) {
294294
const auto op = predicate->op();
@@ -337,8 +337,8 @@ class ProjectionUtil {
337337
// Fixes an inclusive projection to account for incorrectly transformed values.
338338
// align with Java implementation:
339339
// https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java#L275
340-
static Result<std::shared_ptr<UnboundPredicate>> FixInclusiveTimeProjection(
341-
const std::shared_ptr<UnboundPredicateImpl<BoundReference>>& projected) {
340+
static Result<std::unique_ptr<UnboundPredicate>> FixInclusiveTimeProjection(
341+
std::unique_ptr<UnboundPredicateImpl<BoundReference>> projected) {
342342
if (projected == nullptr) {
343343
return nullptr;
344344
}
@@ -382,7 +382,7 @@ class ProjectionUtil {
382382
case Expression::Operation::kGt:
383383
case Expression::Operation::kGtEq:
384384
// incorrect projected values are already greater than the bound for GT, GT_EQ
385-
return projected;
385+
return std::move(projected);
386386

387387
case Expression::Operation::kEq: {
388388
ICEBERG_DCHECK(!projected->literals().empty(), "Expected at least one literal");
@@ -446,7 +446,7 @@ class ProjectionUtil {
446446
}
447447

448448
public:
449-
static Result<std::shared_ptr<UnboundPredicate>> IdentityProject(
449+
static Result<std::unique_ptr<UnboundPredicate>> IdentityProject(
450450
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate) {
451451
ICEBERG_ASSIGN_OR_RAISE(auto ref, NamedReference::Make(std::string(name)));
452452
switch (predicate->kind()) {
@@ -478,7 +478,7 @@ class ProjectionUtil {
478478
std::unreachable();
479479
}
480480

481-
static Result<std::shared_ptr<UnboundPredicate>> BucketProject(
481+
static Result<std::unique_ptr<UnboundPredicate>> BucketProject(
482482
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate,
483483
const std::shared_ptr<TransformFunction>& func) {
484484
ICEBERG_ASSIGN_OR_RAISE(auto ref, NamedReference::Make(std::string(name)));
@@ -519,7 +519,7 @@ class ProjectionUtil {
519519
return nullptr;
520520
}
521521

522-
static Result<std::shared_ptr<UnboundPredicate>> TruncateProject(
522+
static Result<std::unique_ptr<UnboundPredicate>> TruncateProject(
523523
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate,
524524
const std::shared_ptr<TransformFunction>& func) {
525525
ICEBERG_ASSIGN_OR_RAISE(auto ref, NamedReference::Make(std::string(name)));
@@ -561,7 +561,7 @@ class ProjectionUtil {
561561
}
562562
}
563563

564-
static Result<std::shared_ptr<UnboundPredicate>> TemporalProject(
564+
static Result<std::unique_ptr<UnboundPredicate>> TemporalProject(
565565
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate,
566566
const std::shared_ptr<TransformFunction>& func) {
567567
ICEBERG_ASSIGN_OR_RAISE(auto ref, NamedReference::Make(std::string(name)));
@@ -581,7 +581,7 @@ class ProjectionUtil {
581581
auto fixed,
582582
FixInclusiveTimeProjection(
583583
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
584-
projected)));
584+
std::move(projected))));
585585
return fixed;
586586
}
587587
return projected;
@@ -599,13 +599,13 @@ class ProjectionUtil {
599599
auto fixed,
600600
FixInclusiveTimeProjection(
601601
internal::checked_pointer_cast<UnboundPredicateImpl<BoundReference>>(
602-
projected)));
602+
std::move(projected))));
603603
return fixed;
604604
}
605605
return projected;
606606
}
607607

608-
static Result<std::shared_ptr<UnboundPredicate>> RemoveTransform(
608+
static Result<std::unique_ptr<UnboundPredicate>> RemoveTransform(
609609
std::string_view name, const std::shared_ptr<BoundPredicate>& predicate) {
610610
ICEBERG_ASSIGN_OR_RAISE(auto ref, NamedReference::Make(std::string(name)));
611611
switch (predicate->kind()) {

0 commit comments

Comments
 (0)