Skip to content

Commit 9682f60

Browse files
committed
resolve some comments
1 parent 5e1d503 commit 9682f60

2 files changed

Lines changed: 21 additions & 21 deletions

File tree

src/iceberg/partition_spec.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ Status PartitionSpec::ValidatePartitionName(const Schema& schema,
189189
auto name = std::string(partition_field.name());
190190
ICEBERG_CHECK(!name.empty(), "Cannot use empty partition name: {}", name);
191191

192-
ICEBERG_PRECHECK(!partition_names.contains(name),
193-
"Cannot use partition name more than once: {}", name);
192+
ICEBERG_CHECK(!partition_names.contains(name),
193+
"Cannot use partition name more than once: {}", name);
194194
partition_names.insert(name);
195195

196196
ICEBERG_ASSIGN_OR_RAISE(auto schema_field, schema.FindFieldByName(name));

src/iceberg/test/partition_spec_test.cc

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsExactDuplicates) {
311311

312312
// Test: exact duplicate transforms on same field (same dedup name)
313313
{
314-
PartitionField field1(1, 1000, "ts_day1", Transform::Day());
315-
PartitionField field2(1, 1001, "ts_day2", Transform::Day());
314+
PartitionField field1(1, 1000, "ts_day_1_0", Transform::Day());
315+
PartitionField field2(1, 1001, "ts_day_1_1", Transform::Day());
316316

317317
auto result = PartitionSpec::Make(schema, 1, {field1, field2}, false);
318318
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
@@ -322,8 +322,8 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsExactDuplicates) {
322322

323323
// Test: same bucket size on same field (redundant)
324324
{
325-
PartitionField bucket1(2, 1000, "id_bucket1", Transform::Bucket(16));
326-
PartitionField bucket2(2, 1001, "id_bucket2", Transform::Bucket(16));
325+
PartitionField bucket1(2, 1000, "id_bucket_16_2_0", Transform::Bucket(16));
326+
PartitionField bucket2(2, 1001, "id_bucket_16_2_1", Transform::Bucket(16));
327327

328328
auto result = PartitionSpec::Make(schema, 1, {bucket1, bucket2}, false);
329329
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
@@ -335,8 +335,8 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsExactDuplicates) {
335335
auto name_field = SchemaField::MakeRequired(3, "name", string());
336336
Schema schema_with_string({name_field}, Schema::kInitialSchemaId);
337337

338-
PartitionField truncate1(3, 1000, "name_trunc1", Transform::Truncate(4));
339-
PartitionField truncate2(3, 1001, "name_trunc2", Transform::Truncate(4));
338+
PartitionField truncate1(3, 1000, "name_trunc_4_3_1", Transform::Truncate(4));
339+
PartitionField truncate2(3, 1001, "name_trunc_4_3_2", Transform::Truncate(4));
340340

341341
auto result =
342342
PartitionSpec::Make(schema_with_string, 1, {truncate1, truncate2}, false);
@@ -354,35 +354,35 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsAllowedCases) {
354354

355355
// Test: different bucket sizes on same field (allowed - different dedup names)
356356
{
357-
PartitionField bucket16(2, 1000, "id_bucket16", Transform::Bucket(16));
358-
PartitionField bucket32(2, 1001, "id_bucket32", Transform::Bucket(32));
357+
PartitionField bucket16(2, 1000, "id_bucket_16_2", Transform::Bucket(16));
358+
PartitionField bucket32(2, 1001, "id_bucket_32_2", Transform::Bucket(32));
359359

360360
auto result = PartitionSpec::Make(schema, 1, {bucket16, bucket32}, false);
361361
EXPECT_THAT(result, IsOk());
362362
}
363363

364364
// Test: different truncate widths on same field (allowed - different dedup names)
365365
{
366-
PartitionField truncate4(3, 1000, "name_trunc4", Transform::Truncate(4));
367-
PartitionField truncate8(3, 1001, "name_trunc8", Transform::Truncate(8));
366+
PartitionField truncate4(3, 1000, "name_trunc_4_3", Transform::Truncate(4));
367+
PartitionField truncate8(3, 1001, "name_trunc_8_3", Transform::Truncate(8));
368368

369369
auto result = PartitionSpec::Make(schema, 1, {truncate4, truncate8}, false);
370370
EXPECT_THAT(result, IsOk());
371371
}
372372

373373
// Test: same transforms on different fields (allowed)
374374
{
375-
PartitionField ts_day(1, 1000, "ts_day", Transform::Day());
376-
PartitionField id_bucket(2, 1001, "id_bucket", Transform::Bucket(16));
375+
PartitionField ts_day(1, 1000, "ts_day_1", Transform::Day());
376+
PartitionField id_bucket(2, 1001, "id_bucket_2", Transform::Bucket(16));
377377

378378
auto result = PartitionSpec::Make(schema, 1, {ts_day, id_bucket}, false);
379379
EXPECT_THAT(result, IsOk());
380380
}
381381

382382
// Test: different transforms on same field (allowed if dedup names differ)
383383
{
384-
PartitionField ts_day(1, 1000, "ts_day", Transform::Day());
385-
PartitionField ts_month(1, 1001, "ts_month", Transform::Month());
384+
PartitionField ts_day(1, 1000, "ts_day_1", Transform::Day());
385+
PartitionField ts_month(1, 1001, "ts_month_1", Transform::Month());
386386

387387
// This should be allowed since Day and Month have different dedup names
388388
// The Java logic only checks for exact dedup name matches
@@ -392,7 +392,7 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsAllowedCases) {
392392

393393
// Test: single partition field (no redundancy possible)
394394
{
395-
PartitionField single_field(1, 1000, "ts_year", Transform::Year());
395+
PartitionField single_field(1, 1000, "ts_year_1", Transform::Year());
396396

397397
auto result = PartitionSpec::Make(schema, 1, {single_field}, false);
398398
EXPECT_THAT(result, IsOk());
@@ -407,8 +407,8 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsIdentityTransforms) {
407407

408408
// Test: multiple identity transforms on same field (redundant)
409409
{
410-
PartitionField identity1(1, 1000, "id1", Transform::Identity());
411-
PartitionField identity2(1, 1001, "id2", Transform::Identity());
410+
PartitionField identity1(1, 1000, "id_1_0", Transform::Identity());
411+
PartitionField identity2(1, 1001, "id_1_1", Transform::Identity());
412412

413413
auto result = PartitionSpec::Make(schema, 1, {identity1, identity2}, false);
414414
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
@@ -417,8 +417,8 @@ TEST(PartitionSpecTest, ValidateRedundantPartitionsIdentityTransforms) {
417417

418418
// Test: identity transforms on different fields (allowed)
419419
{
420-
PartitionField id_identity(1, 1000, "id", Transform::Identity());
421-
PartitionField name_identity(2, 1001, "name", Transform::Identity());
420+
PartitionField id_identity(1, 1000, "id_1", Transform::Identity());
421+
PartitionField name_identity(2, 1001, "name_2", Transform::Identity());
422422

423423
auto result = PartitionSpec::Make(schema, 1, {id_identity, name_identity}, false);
424424
EXPECT_THAT(result, IsOk());

0 commit comments

Comments
 (0)