Skip to content

Commit 77c5146

Browse files
KontinuationKontinuation
committed
Address review comments
1 parent bb70b83 commit 77c5146

28 files changed

+278
-257
lines changed

‎api/src/main/java/org/apache/iceberg/expressions/BoundGeospatialPredicate.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,17 @@
1919
package org.apache.iceberg.expressions;
2020

2121
import java.nio.ByteBuffer;
22-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
22+
import org.apache.iceberg.geospatial.BoundingBox;
2323

2424
public class BoundGeospatialPredicate extends BoundPredicate<ByteBuffer> {
25-
private final Literal<GeospatialBoundingBox> literal;
25+
private final Literal<BoundingBox> literal;
2626

27-
BoundGeospatialPredicate(
28-
Operation op, BoundTerm<ByteBuffer> term, Literal<GeospatialBoundingBox> literal) {
27+
BoundGeospatialPredicate(Operation op, BoundTerm<ByteBuffer> term, Literal<BoundingBox> literal) {
2928
super(op, term);
3029
this.literal = literal;
3130
}
3231

33-
public Literal<GeospatialBoundingBox> literal() {
32+
public Literal<BoundingBox> literal() {
3433
return literal;
3534
}
3635

‎api/src/main/java/org/apache/iceberg/expressions/Evaluator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import java.util.Set;
2424
import org.apache.iceberg.StructLike;
2525
import org.apache.iceberg.expressions.ExpressionVisitors.BoundVisitor;
26-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
26+
import org.apache.iceberg.geospatial.BoundingBox;
2727
import org.apache.iceberg.types.Types.StructType;
2828
import org.apache.iceberg.util.NaNUtil;
2929

@@ -159,13 +159,13 @@ public <T> Boolean notStartsWith(Bound<T> valueExpr, Literal<T> lit) {
159159
}
160160

161161
@Override
162-
public <T> Boolean stIntersects(Bound<T> valueExpr, Literal<GeospatialBoundingBox> literal) {
162+
public <T> Boolean stIntersects(Bound<T> valueExpr, Literal<BoundingBox> literal) {
163163
throw new UnsupportedOperationException(
164164
"Evaluation of stIntersects against geometry/geography value is not implemented.");
165165
}
166166

167167
@Override
168-
public <T> Boolean stDisjoint(Bound<T> valueExpr, Literal<GeospatialBoundingBox> literal) {
168+
public <T> Boolean stDisjoint(Bound<T> valueExpr, Literal<BoundingBox> literal) {
169169
throw new UnsupportedOperationException(
170170
"Evaluation of stDisjoint against geometry/geography value is not implemented.");
171171
}

‎api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import org.apache.iceberg.PartitionSpec;
3434
import org.apache.iceberg.Schema;
3535
import org.apache.iceberg.Table;
36-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
36+
import org.apache.iceberg.geospatial.BoundingBox;
3737
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
3838
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
3939
import org.apache.iceberg.transforms.Transforms;
@@ -315,7 +315,7 @@ public <T> Expression predicate(BoundPredicate<T> pred) {
315315
} else if (pred.isGeospatialPredicate()) {
316316
BoundGeospatialPredicate bound = (BoundGeospatialPredicate) pred;
317317
return Expressions.geospatialPredicate(
318-
pred.op(), unbind(bound.term()), GeospatialBoundingBox.SANITIZED);
318+
pred.op(), unbind(bound.term()), BoundingBox.empty());
319319
}
320320

321321
throw new UnsupportedOperationException("Cannot sanitize bound predicate type: " + pred.op());
@@ -344,7 +344,7 @@ public <T> Expression predicate(UnboundPredicate<T> pred) {
344344
case ST_INTERSECTS:
345345
case ST_DISJOINT:
346346
return Expressions.geospatialPredicate(
347-
pred.op(), (UnboundTerm<ByteBuffer>) pred.term(), GeospatialBoundingBox.SANITIZED);
347+
pred.op(), (UnboundTerm<ByteBuffer>) pred.term(), BoundingBox.empty());
348348
case IN:
349349
case NOT_IN:
350350
Iterable<T> iter =
@@ -496,9 +496,9 @@ public <T> String predicate(UnboundPredicate<T> pred) {
496496
case NOT_STARTS_WITH:
497497
return term + " NOT STARTS WITH " + sanitize(pred.literal(), nowMicros, today);
498498
case ST_INTERSECTS:
499-
return term + " ST_INTERSECTS WITH " + GeospatialBoundingBox.SANITIZED;
499+
return term + " ST_INTERSECTS WITH (bounding-box)";
500500
case ST_DISJOINT:
501-
return term + " ST_DISJOINT WITH " + GeospatialBoundingBox.SANITIZED;
501+
return term + " ST_DISJOINT WITH (bounding-box)";
502502
default:
503503
throw new UnsupportedOperationException(
504504
"Cannot sanitize unsupported predicate type: " + pred.op());

‎api/src/main/java/org/apache/iceberg/expressions/ExpressionVisitors.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import java.util.Set;
2222
import java.util.function.Supplier;
2323
import org.apache.iceberg.exceptions.ValidationException;
24-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
24+
import org.apache.iceberg.geospatial.BoundingBox;
2525

2626
/** Utils for traversing {@link Expression expressions}. */
2727
public class ExpressionVisitors {
@@ -127,12 +127,12 @@ public <T> R notStartsWith(BoundReference<T> ref, Literal<T> lit) {
127127
"notStartsWith expression is not supported by the visitor");
128128
}
129129

130-
public <T> R stIntersects(BoundReference<T> ref, Literal<GeospatialBoundingBox> lit) {
130+
public <T> R stIntersects(BoundReference<T> ref, Literal<BoundingBox> lit) {
131131
throw new UnsupportedOperationException(
132132
"stIntersects expression is not supported by the visitor");
133133
}
134134

135-
public <T> R stDisjoint(BoundReference<T> ref, Literal<GeospatialBoundingBox> lit) {
135+
public <T> R stDisjoint(BoundReference<T> ref, Literal<BoundingBox> lit) {
136136
throw new UnsupportedOperationException(
137137
"stDisjoint expression is not supported by the visitor");
138138
}
@@ -290,11 +290,11 @@ public <T> R notStartsWith(Bound<T> expr, Literal<T> lit) {
290290
throw new UnsupportedOperationException("Unsupported operation.");
291291
}
292292

293-
public <T> R stIntersects(Bound<T> term, Literal<GeospatialBoundingBox> literal) {
293+
public <T> R stIntersects(Bound<T> term, Literal<BoundingBox> literal) {
294294
throw new UnsupportedOperationException("ST_INTERSECTS is not supported by the visitor");
295295
}
296296

297-
public <T> R stDisjoint(Bound<T> term, Literal<GeospatialBoundingBox> literal) {
297+
public <T> R stDisjoint(Bound<T> term, Literal<BoundingBox> literal) {
298298
throw new UnsupportedOperationException("ST_DISJOINT is not supported by the visitor");
299299
}
300300

@@ -602,11 +602,11 @@ public <T> R notStartsWith(BoundTerm<T> term, Literal<T> lit) {
602602
return null;
603603
}
604604

605-
public <T> R stIntersects(BoundTerm<T> term, Literal<GeospatialBoundingBox> lit) {
605+
public <T> R stIntersects(BoundTerm<T> term, Literal<BoundingBox> lit) {
606606
return null;
607607
}
608608

609-
public <T> R stDisjoint(BoundTerm<T> term, Literal<GeospatialBoundingBox> lit) {
609+
public <T> R stDisjoint(BoundTerm<T> term, Literal<BoundingBox> lit) {
610610
return null;
611611
}
612612
}

‎api/src/main/java/org/apache/iceberg/expressions/Expressions.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import java.nio.ByteBuffer;
2222
import java.util.stream.Stream;
2323
import org.apache.iceberg.expressions.Expression.Operation;
24-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
24+
import org.apache.iceberg.geospatial.BoundingBox;
2525
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
2626
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
2727
import org.apache.iceberg.transforms.Transform;
@@ -204,8 +204,7 @@ public static UnboundPredicate<String> notStartsWith(UnboundTerm<String> expr, S
204204
return new UnboundPredicate<>(Expression.Operation.NOT_STARTS_WITH, expr, value);
205205
}
206206

207-
public static UnboundPredicate<ByteBuffer> stIntersects(
208-
String name, GeospatialBoundingBox value) {
207+
public static UnboundPredicate<ByteBuffer> stIntersects(String name, BoundingBox value) {
209208
return geospatialPredicate(Operation.ST_INTERSECTS, name, value);
210209
}
211210

@@ -215,7 +214,7 @@ public static UnboundPredicate<ByteBuffer> stIntersects(
215214
}
216215

217216
public static UnboundPredicate<ByteBuffer> stIntersects(
218-
UnboundTerm<ByteBuffer> expr, GeospatialBoundingBox value) {
217+
UnboundTerm<ByteBuffer> expr, BoundingBox value) {
219218
return geospatialPredicate(Operation.ST_INTERSECTS, expr, value);
220219
}
221220

@@ -224,7 +223,7 @@ public static UnboundPredicate<ByteBuffer> stIntersects(
224223
return geospatialPredicate(Operation.ST_INTERSECTS, expr, min, max);
225224
}
226225

227-
public static UnboundPredicate<ByteBuffer> stDisjoint(String name, GeospatialBoundingBox value) {
226+
public static UnboundPredicate<ByteBuffer> stDisjoint(String name, BoundingBox value) {
228227
return geospatialPredicate(Operation.ST_DISJOINT, name, value);
229228
}
230229

@@ -234,7 +233,7 @@ public static UnboundPredicate<ByteBuffer> stDisjoint(
234233
}
235234

236235
public static UnboundPredicate<ByteBuffer> stDisjoint(
237-
UnboundTerm<ByteBuffer> expr, GeospatialBoundingBox value) {
236+
UnboundTerm<ByteBuffer> expr, BoundingBox value) {
238237
return geospatialPredicate(Operation.ST_DISJOINT, expr, value);
239238
}
240239

@@ -322,13 +321,13 @@ public static <T> UnboundPredicate<T> predicate(Operation op, UnboundTerm<T> exp
322321
}
323322

324323
public static UnboundPredicate<ByteBuffer> geospatialPredicate(
325-
Operation op, String name, GeospatialBoundingBox value) {
324+
Operation op, String name, BoundingBox value) {
326325
return geospatialPredicate(
327326
op, ref(name), value.min().toByteBuffer(), value.max().toByteBuffer());
328327
}
329328

330329
public static UnboundPredicate<ByteBuffer> geospatialPredicate(
331-
Operation op, UnboundTerm<ByteBuffer> expr, GeospatialBoundingBox value) {
330+
Operation op, UnboundTerm<ByteBuffer> expr, BoundingBox value) {
332331
return geospatialPredicate(op, expr, value.min().toByteBuffer(), value.max().toByteBuffer());
333332
}
334333

‎api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import org.apache.iceberg.ContentFile;
3030
import org.apache.iceberg.DataFile;
3131
import org.apache.iceberg.Schema;
32-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
32+
import org.apache.iceberg.geospatial.BoundingBox;
3333
import org.apache.iceberg.geospatial.GeospatialPredicateEvaluators;
3434
import org.apache.iceberg.transforms.Transform;
3535
import org.apache.iceberg.types.Comparators;
@@ -474,7 +474,7 @@ public <T> Boolean notStartsWith(Bound<T> term, Literal<T> lit) {
474474
}
475475

476476
@Override
477-
public <T> Boolean stIntersects(Bound<T> term, Literal<GeospatialBoundingBox> lit) {
477+
public <T> Boolean stIntersects(Bound<T> term, Literal<BoundingBox> lit) {
478478
T lower = lowerBound(term);
479479
T upper = upperBound(term);
480480

@@ -483,9 +483,8 @@ public <T> Boolean stIntersects(Bound<T> term, Literal<GeospatialBoundingBox> li
483483
}
484484

485485
if (lit.value() != null && lower instanceof ByteBuffer && upper instanceof ByteBuffer) {
486-
GeospatialBoundingBox dataBox =
487-
GeospatialBoundingBox.fromByteBuffers((ByteBuffer) lower, (ByteBuffer) upper);
488-
GeospatialBoundingBox queryBox = lit.value();
486+
BoundingBox dataBox = BoundingBox.fromByteBuffers((ByteBuffer) lower, (ByteBuffer) upper);
487+
BoundingBox queryBox = lit.value();
489488

490489
// If the data box and query box doesn't intersect, no records can match
491490
GeospatialPredicateEvaluators.GeospatialPredicateEvaluator evaluator =
@@ -499,7 +498,7 @@ public <T> Boolean stIntersects(Bound<T> term, Literal<GeospatialBoundingBox> li
499498
}
500499

501500
@Override
502-
public <T> Boolean stDisjoint(Bound<T> term, Literal<GeospatialBoundingBox> lit) {
501+
public <T> Boolean stDisjoint(Bound<T> term, Literal<BoundingBox> lit) {
503502
return ROWS_MIGHT_MATCH;
504503
}
505504

‎api/src/main/java/org/apache/iceberg/expressions/Literal.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import java.nio.ByteBuffer;
2424
import java.util.Comparator;
2525
import java.util.UUID;
26-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
26+
import org.apache.iceberg.geospatial.BoundingBox;
2727
import org.apache.iceberg.types.Type;
2828

2929
/**
@@ -72,7 +72,7 @@ static Literal<BigDecimal> of(BigDecimal value) {
7272
return new Literals.DecimalLiteral(value);
7373
}
7474

75-
static Literal<GeospatialBoundingBox> of(GeospatialBoundingBox value) {
75+
static Literal<BoundingBox> of(BoundingBox value) {
7676
return new Literals.GeospatialBoundingBoxLiteral(value);
7777
}
7878

‎api/src/main/java/org/apache/iceberg/expressions/Literals.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import java.util.Comparator;
3333
import java.util.Objects;
3434
import java.util.UUID;
35-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
35+
import org.apache.iceberg.geospatial.BoundingBox;
3636
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
3737
import org.apache.iceberg.relocated.com.google.common.io.BaseEncoding;
3838
import org.apache.iceberg.types.Comparators;
@@ -81,8 +81,8 @@ static <T> Literal<T> from(T value) {
8181
return (Literal<T>) new Literals.BinaryLiteral((ByteBuffer) value);
8282
} else if (value instanceof BigDecimal) {
8383
return (Literal<T>) new Literals.DecimalLiteral((BigDecimal) value);
84-
} else if (value instanceof GeospatialBoundingBox) {
85-
return (Literal<T>) new Literals.GeospatialBoundingBoxLiteral((GeospatialBoundingBox) value);
84+
} else if (value instanceof BoundingBox) {
85+
return (Literal<T>) new Literals.GeospatialBoundingBoxLiteral((BoundingBox) value);
8686
}
8787

8888
throw new IllegalArgumentException(
@@ -691,18 +691,18 @@ public String toString() {
691691
}
692692
}
693693

694-
static class GeospatialBoundingBoxLiteral implements Literal<GeospatialBoundingBox> {
695-
private static final Comparator<GeospatialBoundingBox> CMP =
696-
Comparators.<GeospatialBoundingBox>nullsFirst().thenComparing(Comparator.naturalOrder());
694+
static class GeospatialBoundingBoxLiteral implements Literal<BoundingBox> {
695+
private static final Comparator<BoundingBox> CMP =
696+
Comparators.<BoundingBox>nullsFirst().thenComparing(Comparator.naturalOrder());
697697

698-
private final GeospatialBoundingBox value;
698+
private final BoundingBox value;
699699

700-
GeospatialBoundingBoxLiteral(GeospatialBoundingBox value) {
700+
GeospatialBoundingBoxLiteral(BoundingBox value) {
701701
this.value = value;
702702
}
703703

704704
@Override
705-
public GeospatialBoundingBox value() {
705+
public BoundingBox value() {
706706
return value;
707707
}
708708

@@ -712,7 +712,7 @@ public <T> Literal<T> to(Type type) {
712712
}
713713

714714
@Override
715-
public Comparator<GeospatialBoundingBox> comparator() {
715+
public Comparator<BoundingBox> comparator() {
716716
return CMP;
717717
}
718718

‎api/src/main/java/org/apache/iceberg/expressions/StrictMetricsEvaluator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import org.apache.iceberg.DataFile;
3030
import org.apache.iceberg.Schema;
3131
import org.apache.iceberg.expressions.ExpressionVisitors.BoundExpressionVisitor;
32-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
32+
import org.apache.iceberg.geospatial.BoundingBox;
3333
import org.apache.iceberg.types.Conversions;
3434
import org.apache.iceberg.types.Types.StructType;
3535
import org.apache.iceberg.util.NaNUtil;
@@ -474,12 +474,12 @@ public <T> Boolean notStartsWith(BoundReference<T> ref, Literal<T> lit) {
474474
}
475475

476476
@Override
477-
public <T> Boolean stIntersects(BoundReference<T> ref, Literal<GeospatialBoundingBox> lit) {
477+
public <T> Boolean stIntersects(BoundReference<T> ref, Literal<BoundingBox> lit) {
478478
return ROWS_MIGHT_NOT_MATCH;
479479
}
480480

481481
@Override
482-
public <T> Boolean stDisjoint(BoundReference<T> ref, Literal<GeospatialBoundingBox> lit) {
482+
public <T> Boolean stDisjoint(BoundReference<T> ref, Literal<BoundingBox> lit) {
483483
return ROWS_MIGHT_NOT_MATCH;
484484
}
485485

‎api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import java.util.List;
2323
import java.util.Set;
2424
import org.apache.iceberg.exceptions.ValidationException;
25-
import org.apache.iceberg.geospatial.GeospatialBoundingBox;
25+
import org.apache.iceberg.geospatial.BoundingBox;
2626
import org.apache.iceberg.relocated.com.google.common.base.Joiner;
2727
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
2828
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
@@ -282,7 +282,7 @@ private Expression bindGeospatialOperation(BoundTerm<T> boundTerm) {
282282
return new BoundGeospatialPredicate(
283283
op(),
284284
(BoundTerm<ByteBuffer>) boundTerm,
285-
Literals.from(GeospatialBoundingBox.fromByteBuffers(min.value(), max.value())));
285+
Literals.from(BoundingBox.fromByteBuffers(min.value(), max.value())));
286286
}
287287

288288
@Override
@@ -319,7 +319,7 @@ public String toString() {
319319
String opName = op() == Operation.ST_INTERSECTS ? " stIntersects " : " stDisjoint ";
320320
return term()
321321
+ opName
322-
+ GeospatialBoundingBox.fromByteBuffers(minLiteral.value(), maxLiteral.value());
322+
+ BoundingBox.fromByteBuffers(minLiteral.value(), maxLiteral.value());
323323
case IN:
324324
return term() + " in (" + COMMA.join(literals()) + ")";
325325
case NOT_IN:

0 commit comments

Comments
 (0)