Skip to content

Commit a977a83

Browse files
sfc-gh-prsinghsfc-gh-prsingh
committed
Address review code
1 parent e68fcd5 commit a977a83

File tree

3 files changed

+44
-47
lines changed

3 files changed

+44
-47
lines changed

‎core/src/test/java/org/apache/iceberg/TestBase.java‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,15 @@ public class TestBase {
9191
.withPartitionPath("data_bucket=0") // easy way to set partition data for now
9292
.withRecordCount(1)
9393
.build();
94+
public static final DeleteFile FILE_A_EQUALITY_DELETES =
95+
FileMetadata.deleteFileBuilder(SPEC)
96+
.ofEqualityDeletes(1) // delete on column 1 (id column)
97+
.withPath("/path/to/data-a-equality-deletes.parquet")
98+
.withFileSizeInBytes(10)
99+
.withPartitionPath("data_bucket=0") // easy way to set partition data for now
100+
.withRecordCount(1)
101+
.build();
102+
94103
static final DeleteFile FILE_A_DV =
95104
FileMetadata.deleteFileBuilder(SPEC)
96105
.ofPositionDeletes()
@@ -127,6 +136,14 @@ public class TestBase {
127136
.withPartitionPath("data_bucket=1") // easy way to set partition data for now
128137
.withRecordCount(1)
129138
.build();
139+
public static final DeleteFile FILE_B_EQUALITY_DELETES =
140+
FileMetadata.deleteFileBuilder(SPEC)
141+
.ofEqualityDeletes(1) // delete on column 1 (id column)
142+
.withPath("/path/to/data-b-equality-deletes.parquet")
143+
.withFileSizeInBytes(10)
144+
.withPartitionPath("data_bucket=1") // same partition as FILE_B
145+
.withRecordCount(1)
146+
.build();
130147
static final DeleteFile FILE_B_DV =
131148
FileMetadata.deleteFileBuilder(SPEC)
132149
.ofPositionDeletes()
@@ -154,6 +171,14 @@ public class TestBase {
154171
.withPartitionPath("data_bucket=2") // easy way to set partition data for now
155172
.withRecordCount(1)
156173
.build();
174+
public static final DeleteFile FILE_C_EQUALITY_DELETES =
175+
FileMetadata.deleteFileBuilder(SPEC)
176+
.ofEqualityDeletes(1) // delete on column 1 (id column)
177+
.withPath("/path/to/data-c-equality-deletes.parquet")
178+
.withFileSizeInBytes(10)
179+
.withPartitionPath("data_bucket=2") // same partition as FILE_C
180+
.withRecordCount(1)
181+
.build();
157182
static final DataFile FILE_D =
158183
DataFiles.builder(SPEC)
159184
.withPath("/path/to/data-d.parquet")
@@ -170,7 +195,7 @@ public class TestBase {
170195
.withPartitionPath("data_bucket=3") // easy way to set partition data for now
171196
.withRecordCount(1)
172197
.build();
173-
static final DataFile FILE_WITH_STATS =
198+
public static final DataFile FILE_WITH_STATS =
174199
DataFiles.builder(SPEC)
175200
.withPath("/path/to/data-with-stats.parquet")
176201
.withMetrics(

‎core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@
8888
public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
8989
private static final String BASE_TABLE_LOCATION = "file:/tmp";
9090
protected static final Namespace NS = Namespace.of("newdb");
91-
public static final TableIdentifier TABLE = TableIdentifier.of(NS, "newtable");
91+
protected static final TableIdentifier TABLE = TableIdentifier.of(NS, "newtable");
9292
protected static final TableIdentifier RENAMED_TABLE = TableIdentifier.of(NS, "table_renamed");
9393
protected static final TableIdentifier TBL = TableIdentifier.of("ns", "tbl");
9494

@@ -115,8 +115,7 @@ public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
115115
new Schema(required(1, "some_id", Types.IntegerType.get()));
116116

117117
// Partition spec used to create tables
118-
public static final PartitionSpec SPEC =
119-
PartitionSpec.builderFor(SCHEMA).bucket("id", 16).build();
118+
static final PartitionSpec SPEC = PartitionSpec.builderFor(SCHEMA).bucket("id", 16).build();
120119

121120
protected static final PartitionSpec TABLE_SPEC =
122121
PartitionSpec.builderFor(TABLE_SCHEMA).bucket("id", 16).build();
@@ -150,7 +149,7 @@ public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
150149
.withRecordCount(2) // needs at least one record or else metrics will filter it out
151150
.build();
152151

153-
public static final DataFile FILE_C =
152+
static final DataFile FILE_C =
154153
DataFiles.builder(SPEC)
155154
.withPath("/path/to/data-c.parquet")
156155
.withFileSizeInBytes(10)
@@ -159,7 +158,7 @@ public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
159158
.build();
160159

161160
// Delete files for testing
162-
public static final DeleteFile FILE_A_DELETES =
161+
static final DeleteFile FILE_A_DELETES =
163162
FileMetadata.deleteFileBuilder(SPEC)
164163
.ofPositionDeletes()
165164
.withPath("/path/to/data-a-deletes.parquet")
@@ -168,16 +167,7 @@ public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
168167
.withRecordCount(1)
169168
.build();
170169

171-
public static final DeleteFile FILE_A_EQUALITY_DELETES =
172-
FileMetadata.deleteFileBuilder(SPEC)
173-
.ofEqualityDeletes(1) // delete on column 1 (id column)
174-
.withPath("/path/to/data-a-equality-deletes.parquet")
175-
.withFileSizeInBytes(10)
176-
.withPartitionPath("id_bucket=0") // same partition as FILE_A
177-
.withRecordCount(1)
178-
.build();
179-
180-
public static final DeleteFile FILE_B_DELETES =
170+
static final DeleteFile FILE_B_DELETES =
181171
FileMetadata.deleteFileBuilder(SPEC)
182172
.ofPositionDeletes()
183173
.withPath("/path/to/data-b-deletes.parquet")
@@ -186,24 +176,6 @@ public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
186176
.withRecordCount(1)
187177
.build();
188178

189-
public static final DeleteFile FILE_B_EQUALITY_DELETES =
190-
FileMetadata.deleteFileBuilder(SPEC)
191-
.ofEqualityDeletes(1) // delete on column 1 (id column)
192-
.withPath("/path/to/data-b-equality-deletes.parquet")
193-
.withFileSizeInBytes(10)
194-
.withPartitionPath("id_bucket=1") // same partition as FILE_B
195-
.withRecordCount(1)
196-
.build();
197-
198-
public static final DeleteFile FILE_C_EQUALITY_DELETES =
199-
FileMetadata.deleteFileBuilder(SPEC)
200-
.ofEqualityDeletes(1) // delete on column 1 (id column)
201-
.withPath("/path/to/data-c-equality-deletes.parquet")
202-
.withFileSizeInBytes(10)
203-
.withPartitionPath("id_bucket=2") // same partition as FILE_C
204-
.withRecordCount(1)
205-
.build();
206-
207179
protected abstract C catalog();
208180

209181
protected abstract C initCatalog(String catalogName, Map<String, String> additionalProperties);

‎core/src/test/java/org/apache/iceberg/rest/TestRESTScanPlanning.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@
1818
*/
1919
package org.apache.iceberg.rest;
2020

21-
import static org.apache.iceberg.catalog.CatalogTests.FILE_A;
22-
import static org.apache.iceberg.catalog.CatalogTests.FILE_A_DELETES;
23-
import static org.apache.iceberg.catalog.CatalogTests.FILE_A_EQUALITY_DELETES;
24-
import static org.apache.iceberg.catalog.CatalogTests.FILE_B;
25-
import static org.apache.iceberg.catalog.CatalogTests.FILE_B_DELETES;
26-
import static org.apache.iceberg.catalog.CatalogTests.FILE_B_EQUALITY_DELETES;
27-
import static org.apache.iceberg.catalog.CatalogTests.FILE_C;
28-
import static org.apache.iceberg.catalog.CatalogTests.FILE_C_EQUALITY_DELETES;
29-
import static org.apache.iceberg.catalog.CatalogTests.SCHEMA;
30-
import static org.apache.iceberg.catalog.CatalogTests.SPEC;
21+
import static org.apache.iceberg.TestBase.FILE_A;
22+
import static org.apache.iceberg.TestBase.FILE_A_DELETES;
23+
import static org.apache.iceberg.TestBase.FILE_A_EQUALITY_DELETES;
24+
import static org.apache.iceberg.TestBase.FILE_B;
25+
import static org.apache.iceberg.TestBase.FILE_B_DELETES;
26+
import static org.apache.iceberg.TestBase.FILE_B_EQUALITY_DELETES;
27+
import static org.apache.iceberg.TestBase.FILE_C;
28+
import static org.apache.iceberg.TestBase.FILE_C_EQUALITY_DELETES;
29+
import static org.apache.iceberg.TestBase.SCHEMA;
30+
import static org.apache.iceberg.TestBase.SPEC;
3131
import static org.assertj.core.api.Assertions.assertThat;
3232
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3333
import static org.assertj.core.api.Assumptions.assumeThat;
@@ -645,7 +645,7 @@ void remoteScanPlanningWithDeletesAndFiltering(
645645

646646
// Create a filtered scan and execute scan planning with filtering and deletes
647647
try (CloseableIterable<FileScanTask> iterable =
648-
table.newScan().filter(Expressions.equal("data", "test")).planFiles()) {
648+
table.newScan().filter(Expressions.lessThan("id", 4)).planFiles()) {
649649
List<FileScanTask> tasks = Lists.newArrayList(iterable);
650650

651651
// Verify scan planning works with both filtering and deletes
@@ -784,7 +784,7 @@ public void scanPlanningWithMultiplePartitionSpecs() throws IOException {
784784
table.newFastAppend().appendFile(FILE_B).commit();
785785

786786
// Evolve partition spec to bucket by id with 8 buckets instead of 16
787-
table.updateSpec().removeField("id_bucket").addField(Expressions.bucket("id", 8)).commit();
787+
table.updateSpec().removeField("data_bucket").addField(Expressions.bucket("data", 8)).commit();
788788

789789
// Create data file with new partition spec (spec-id=1)
790790
PartitionSpec newSpec = table.spec();
@@ -794,7 +794,7 @@ public void scanPlanningWithMultiplePartitionSpecs() throws IOException {
794794
DataFiles.builder(newSpec)
795795
.withPath("/path/to/data-new-spec.parquet")
796796
.withFileSizeInBytes(10)
797-
.withPartitionPath("id_bucket_8=3") // 8-bucket partition
797+
.withPartitionPath("data_bucket_8=3") // 8-bucket partition
798798
.withRecordCount(2)
799799
.build();
800800

0 commit comments

Comments
 (0)