Skip to content

Commit 8a562d6

Browse files
committed
fix comments
1 parent b9c8f09 commit 8a562d6

File tree

6 files changed

+84
-157
lines changed

6 files changed

+84
-157
lines changed

src/iceberg/metrics_config.cc

Lines changed: 29 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "iceberg/table_properties.h"
3131
#include "iceberg/util/checked_cast.h"
3232
#include "iceberg/util/type_util.h"
33+
#include "iceberg/util/visit_type.h"
3334

3435
namespace iceberg {
3536

@@ -41,12 +42,12 @@ constexpr std::string_view kFullName = "full";
4142
constexpr std::string_view kTruncatePrefix = "truncate(";
4243
constexpr int32_t kDefaultTruncateLength = 16;
4344
const std::shared_ptr<MetricsMode> kDefaultMetricsMode =
44-
std::make_shared<TruncateMetricsMode>(kDefaultTruncateLength);
45+
std::make_shared<MetricsMode>(MetricsMode::Kind::kTruncate, kDefaultTruncateLength);
4546

4647
std::shared_ptr<MetricsMode> SortedColumnDefaultMode(
4748
std::shared_ptr<MetricsMode> default_mode) {
48-
if (default_mode->kind() == MetricsMode::Kind::kNone ||
49-
default_mode->kind() == MetricsMode::Kind::kCounts) {
49+
if (default_mode->kind == MetricsMode::Kind::kNone ||
50+
default_mode->kind == MetricsMode::Kind::kCounts) {
5051
return kDefaultMetricsMode;
5152
} else {
5253
return std::move(default_mode);
@@ -74,26 +75,21 @@ Result<std::shared_ptr<MetricsMode>> ParseMode(const std::string& mode,
7475
} // namespace
7576

7677
const std::shared_ptr<MetricsMode>& MetricsMode::None() {
77-
static const std::shared_ptr<MetricsMode> none = std::make_shared<NoneMetricsMode>();
78+
static const auto none = std::make_shared<MetricsMode>(Kind::kNone);
7879
return none;
7980
}
8081

8182
const std::shared_ptr<MetricsMode>& MetricsMode::Counts() {
82-
static const std::shared_ptr<MetricsMode> counts =
83-
std::make_shared<CountsMetricsMode>();
83+
static const auto counts = std::make_shared<MetricsMode>(Kind::kCounts);
8484
return counts;
8585
}
8686

8787
const std::shared_ptr<MetricsMode>& MetricsMode::Full() {
88-
static const std::shared_ptr<MetricsMode> full = std::make_shared<FullMetricsMode>();
88+
static const auto full = std::make_shared<MetricsMode>(Kind::kFull);
8989
return full;
9090
}
9191

92-
const std::shared_ptr<MetricsMode>& MetricsMode::Truncate() {
93-
return kDefaultMetricsMode;
94-
}
95-
96-
Result<std::shared_ptr<MetricsMode>> MetricsMode::FromString(const std::string& mode) {
92+
Result<std::shared_ptr<MetricsMode>> MetricsMode::FromString(std::string_view mode) {
9793
if (StringUtils::EqualsIgnoreCase(mode, kNoneName)) {
9894
return MetricsMode::None();
9995
} else if (StringUtils::EqualsIgnoreCase(mode, kCountsName)) {
@@ -112,42 +108,29 @@ Result<std::shared_ptr<MetricsMode>> MetricsMode::FromString(const std::string&
112108
if (length == kDefaultTruncateLength) {
113109
return kDefaultMetricsMode;
114110
}
115-
return TruncateMetricsMode::Make(length);
111+
ICEBERG_PRECHECK(length > 0, "Truncate length should be positive.");
112+
return std::make_shared<MetricsMode>(Kind::kTruncate, length);
116113
}
117114
return InvalidArgument("Invalid metrics mode: {}", mode);
118115
}
119116

120-
std::string NoneMetricsMode::ToString() const { return std::string(kNoneName); }
121-
std::string CountsMetricsMode::ToString() const { return std::string(kCountsName); }
122-
std::string FullMetricsMode::ToString() const { return std::string(kFullName); }
123-
std::string TruncateMetricsMode::ToString() const {
124-
return std::format("truncate({})", length_);
125-
}
126-
127-
Result<std::shared_ptr<MetricsMode>> TruncateMetricsMode::Make(int32_t length) {
128-
ICEBERG_PRECHECK(length > 0, "Truncate length should be positive.");
129-
return std::make_shared<TruncateMetricsMode>(length);
130-
}
131-
132117
MetricsConfig::MetricsConfig(
133118
std::unordered_map<std::string, std::shared_ptr<MetricsMode>> column_modes,
134119
std::shared_ptr<MetricsMode> default_mode)
135120
: column_modes_(std::move(column_modes)), default_mode_(std::move(default_mode)) {}
136121

137122
const std::shared_ptr<MetricsConfig>& MetricsConfig::Default() {
138-
static const auto default_config = std::make_shared<MetricsConfig>(
139-
std::unordered_map<std::string, std::shared_ptr<MetricsMode>>{},
140-
kDefaultMetricsMode);
123+
static const std::shared_ptr<MetricsConfig> default_config(
124+
new MetricsConfig({}, kDefaultMetricsMode));
141125
return default_config;
142126
}
143127

144-
Result<std::shared_ptr<MetricsConfig>> MetricsConfig::Make(std::shared_ptr<Table> table) {
145-
ICEBERG_PRECHECK(table != nullptr, "table cannot be null");
146-
ICEBERG_ASSIGN_OR_RAISE(auto schema, table->schema());
128+
Result<std::shared_ptr<MetricsConfig>> MetricsConfig::Make(const Table& table) {
129+
ICEBERG_ASSIGN_OR_RAISE(auto schema, table.schema());
147130

148-
auto sort_order = table->sort_order();
131+
auto sort_order = table.sort_order();
149132
return MakeInternal(
150-
table->properties(), *schema,
133+
table.properties(), *schema,
151134
sort_order.has_value() ? *sort_order.value() : *SortOrder::Unsorted());
152135
}
153136

@@ -197,8 +180,8 @@ Result<std::shared_ptr<MetricsConfig>> MetricsConfig::MakeInternal(
197180
}
198181
}
199182

200-
return std::make_shared<MetricsConfig>(std::move(column_modes),
201-
std::move(default_mode));
183+
return std::shared_ptr<MetricsConfig>(
184+
new MetricsConfig(std::move(column_modes), std::move(default_mode)));
202185
}
203186

204187
Result<std::unordered_set<int32_t>> MetricsConfig::LimitFieldIds(const Schema& schema,
@@ -207,31 +190,29 @@ Result<std::unordered_set<int32_t>> MetricsConfig::LimitFieldIds(const Schema& s
207190
public:
208191
explicit Visitor(int32_t limit) : limit_(limit) {}
209192

210-
Status Visit(const std::shared_ptr<Type>& type) {
211-
if (type->is_nested()) {
212-
return Visit(internal::checked_cast<const NestedType&>(*type));
213-
}
214-
return {};
215-
}
193+
Status Visit(const Type& type) { return VisitNestedType(type, this); }
216194

217-
Status Visit(const NestedType& type) {
195+
Status VisitNested(const NestedType& type) {
218196
for (auto& field : type.fields()) {
219197
if (!ShouldContinue()) {
220198
break;
221199
}
200+
// TODO(zhuo.wang) or is_variant
222201
if (field.type()->is_primitive()) {
223202
ids_.insert(field.field_id());
224203
}
225204
}
226205

227206
for (auto& field : type.fields()) {
228207
if (ShouldContinue()) {
229-
ICEBERG_RETURN_UNEXPECTED(Visit(field.type()));
208+
ICEBERG_RETURN_UNEXPECTED(Visit(*field.type()));
230209
}
231210
}
232211
return {};
233212
}
234213

214+
Status VisitNonNested(const Type& type) { return {}; }
215+
235216
std::unordered_set<int32_t> Finish() { return ids_; }
236217

237218
private:
@@ -243,8 +224,7 @@ Result<std::unordered_set<int32_t>> MetricsConfig::LimitFieldIds(const Schema& s
243224
};
244225

245226
Visitor visitor(limit);
246-
ICEBERG_RETURN_UNEXPECTED(
247-
visitor.Visit(internal::checked_cast<const NestedType&>(schema)));
227+
ICEBERG_RETURN_UNEXPECTED(visitor.Visit(internal::checked_cast<const Type&>(schema)));
248228
return visitor.Finish();
249229
}
250230

@@ -257,12 +237,10 @@ Status MetricsConfig::VerifyReferencedColumns(
257237
auto field_name =
258238
std::string_view(key).substr(TableProperties::kMetricModeColumnConfPrefix.size());
259239
ICEBERG_ASSIGN_OR_RAISE(auto field, schema.FindFieldByName(field_name));
260-
if (!field.has_value()) {
261-
return ValidationFailed(
262-
"Invalid metrics config, could not find column {} from table prop {} in "
263-
"schema {}",
264-
field_name, key, schema.ToString());
265-
}
240+
ICEBERG_CHECK(field.has_value(),
241+
"Invalid metrics config, could not find column {} from table prop {} "
242+
"in schema {}",
243+
field_name, key, schema.ToString());
266244
}
267245
return {};
268246
}

src/iceberg/metrics_config.h

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,18 @@
2424

2525
#include <memory>
2626
#include <string>
27+
#include <string_view>
2728
#include <unordered_map>
2829
#include <unordered_set>
30+
#include <variant>
2931

3032
#include "iceberg/iceberg_export.h"
3133
#include "iceberg/result.h"
3234
#include "iceberg/type_fwd.h"
33-
#include "iceberg/util/formattable.h"
3435

3536
namespace iceberg {
3637

37-
class ICEBERG_EXPORT MetricsMode : public util::Formattable {
38+
struct ICEBERG_EXPORT MetricsMode {
3839
public:
3940
enum class Kind : uint8_t {
4041
kNone,
@@ -43,66 +44,24 @@ class ICEBERG_EXPORT MetricsMode : public util::Formattable {
4344
kFull,
4445
};
4546

46-
static Result<std::shared_ptr<MetricsMode>> FromString(const std::string& mode);
47+
static Result<std::shared_ptr<MetricsMode>> FromString(std::string_view mode);
4748

4849
static const std::shared_ptr<MetricsMode>& None();
4950
static const std::shared_ptr<MetricsMode>& Counts();
50-
static const std::shared_ptr<MetricsMode>& Truncate();
5151
static const std::shared_ptr<MetricsMode>& Full();
5252

53-
/// \brief Return the kind of this metrics mode.
54-
virtual Kind kind() const = 0;
55-
56-
std::string ToString() const override = 0;
57-
};
58-
59-
class ICEBERG_EXPORT NoneMetricsMode : public MetricsMode {
60-
public:
61-
constexpr Kind kind() const override { return Kind::kNone; }
62-
63-
std::string ToString() const override;
64-
};
65-
66-
class ICEBERG_EXPORT CountsMetricsMode : public MetricsMode {
67-
public:
68-
constexpr Kind kind() const override { return Kind::kCounts; }
69-
70-
std::string ToString() const override;
71-
};
72-
73-
class ICEBERG_EXPORT TruncateMetricsMode : public MetricsMode {
74-
public:
75-
explicit TruncateMetricsMode(int32_t length) : length_(length) {}
76-
77-
constexpr Kind kind() const override { return Kind::kTruncate; }
78-
79-
std::string ToString() const override;
80-
81-
static Result<std::shared_ptr<MetricsMode>> Make(int32_t length);
82-
83-
private:
84-
const int32_t length_;
85-
};
86-
87-
class ICEBERG_EXPORT FullMetricsMode : public MetricsMode {
88-
public:
89-
constexpr Kind kind() const override { return Kind::kFull; }
90-
91-
std::string ToString() const override;
53+
Kind kind;
54+
std::variant<std::monostate, int32_t> length;
9255
};
9356

9457
/// \brief Configuration utilities for table metrics
9558
class ICEBERG_EXPORT MetricsConfig {
9659
public:
97-
MetricsConfig(
98-
std::unordered_map<std::string, std::shared_ptr<MetricsMode>> column_modes,
99-
std::shared_ptr<MetricsMode> default_mode);
100-
10160
/// \brief Get the default metrics config.
10261
static const std::shared_ptr<MetricsConfig>& Default();
10362

10463
/// \brief Creates a metrics config from a table.
105-
static Result<std::shared_ptr<MetricsConfig>> Make(std::shared_ptr<Table> table);
64+
static Result<std::shared_ptr<MetricsConfig>> Make(const Table& table);
10665

10766
/// \brief Get `limit` num of primitive field ids from schema
10867
static Result<std::unordered_set<int32_t>> LimitFieldIds(const Schema& schema,
@@ -121,6 +80,10 @@ class ICEBERG_EXPORT MetricsConfig {
12180
std::shared_ptr<MetricsMode> ColumnMode(const std::string& column_name) const;
12281

12382
private:
83+
MetricsConfig(
84+
std::unordered_map<std::string, std::shared_ptr<MetricsMode>> column_modes,
85+
std::shared_ptr<MetricsMode> default_mode);
86+
12487
/// \brief Generate a MetricsConfig for all columns based on overrides, schema, and sort
12588
/// order.
12689
///

0 commit comments

Comments
 (0)