Skip to content

Commit f6c0ac7

Browse files
Address review feeback
1 parent 35ae73a commit f6c0ac7

2 files changed

Lines changed: 36 additions & 74 deletions

File tree

core/src/main/java/org/apache/iceberg/SingleValueParser.java

Lines changed: 32 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,41 @@ public static Object fromJson(Type type, JsonNode defaultValue) {
119119
}
120120
return uuid;
121121
case DATE:
122-
return parseDateValue(type, defaultValue);
122+
Preconditions.checkArgument(
123+
defaultValue.isTextual(), "Cannot parse default as a %s value: %s", type, defaultValue);
124+
return DateTimeUtil.isoDateToDays(defaultValue.textValue());
123125
case TIME:
124-
return parseTimeValue(type, defaultValue);
126+
Preconditions.checkArgument(
127+
defaultValue.isTextual(), "Cannot parse default as a %s value: %s", type, defaultValue);
128+
return DateTimeUtil.isoTimeToMicros(defaultValue.textValue());
125129
case TIMESTAMP:
126-
return parseTimestampValue(type, defaultValue);
130+
Preconditions.checkArgument(
131+
defaultValue.isTextual(), "Cannot parse default as a %s value: %s", type, defaultValue);
132+
if (((Types.TimestampType) type).shouldAdjustToUTC()) {
133+
String timestampTz = defaultValue.textValue();
134+
Preconditions.checkArgument(
135+
DateTimeUtil.isUTCTimestamptz(timestampTz),
136+
"Cannot parse default as a %s value: %s, offset must be +00:00",
137+
type,
138+
defaultValue);
139+
return DateTimeUtil.isoTimestamptzToMicros(timestampTz);
140+
} else {
141+
return DateTimeUtil.isoTimestampToMicros(defaultValue.textValue());
142+
}
127143
case TIMESTAMP_NANO:
128-
return parseTimestampNanoValue(type, defaultValue);
129-
144+
Preconditions.checkArgument(
145+
defaultValue.isTextual(), "Cannot parse default as a %s value: %s", type, defaultValue);
146+
if (((Types.TimestampNanoType) type).shouldAdjustToUTC()) {
147+
String timestampTzNano = defaultValue.textValue();
148+
Preconditions.checkArgument(
149+
DateTimeUtil.isUTCTimestamptz(timestampTzNano),
150+
"Cannot parse default as a %s value: %s, offset must be +00:00",
151+
type,
152+
defaultValue);
153+
return DateTimeUtil.isoTimestamptzToNanos(timestampTzNano);
154+
} else {
155+
return DateTimeUtil.isoTimestampToNanos(defaultValue.textValue());
156+
}
130157
case FIXED:
131158
Preconditions.checkArgument(
132159
defaultValue.isTextual(), "Cannot parse default as a %s value: %s", type, defaultValue);
@@ -387,68 +414,4 @@ public static void toJson(Type type, Object defaultValue, JsonGenerator generato
387414
throw new UnsupportedOperationException(String.format("Type: %s is not supported", type));
388415
}
389416
}
390-
391-
private static Object parseDateValue(Type type, JsonNode value) {
392-
if (value.isTextual()) {
393-
return DateTimeUtil.isoDateToDays(value.textValue());
394-
} else if (value.isIntegralNumber() && value.canConvertToInt()) {
395-
return value.intValue();
396-
} else {
397-
throw new IllegalArgumentException(
398-
String.format("Cannot parse default as a %s value: %s", type, value));
399-
}
400-
}
401-
402-
private static Object parseTimeValue(Type type, JsonNode value) {
403-
if (value.isTextual()) {
404-
return DateTimeUtil.isoTimeToMicros(value.textValue());
405-
} else if (value.isIntegralNumber() && value.canConvertToLong()) {
406-
return value.longValue();
407-
} else {
408-
throw new IllegalArgumentException(
409-
String.format("Cannot parse default as a %s value: %s", type, value));
410-
}
411-
}
412-
413-
private static Object parseTimestampValue(Type type, JsonNode value) {
414-
if (value.isTextual()) {
415-
if (((Types.TimestampType) type).shouldAdjustToUTC()) {
416-
String timestampTz = value.textValue();
417-
Preconditions.checkArgument(
418-
DateTimeUtil.isUTCTimestamptz(timestampTz),
419-
"Cannot parse default as a %s value: %s, offset must be +00:00",
420-
type,
421-
value);
422-
return DateTimeUtil.isoTimestamptzToMicros(timestampTz);
423-
} else {
424-
return DateTimeUtil.isoTimestampToMicros(value.textValue());
425-
}
426-
} else if (value.isIntegralNumber() && value.canConvertToLong()) {
427-
return value.longValue();
428-
} else {
429-
throw new IllegalArgumentException(
430-
String.format("Cannot parse default as a %s value: %s", type, value));
431-
}
432-
}
433-
434-
private static Object parseTimestampNanoValue(Type type, JsonNode value) {
435-
if (value.isTextual()) {
436-
if (((Types.TimestampNanoType) type).shouldAdjustToUTC()) {
437-
String timestampTzNano = value.textValue();
438-
Preconditions.checkArgument(
439-
DateTimeUtil.isUTCTimestamptz(timestampTzNano),
440-
"Cannot parse default as a %s value: %s, offset must be +00:00",
441-
type,
442-
value);
443-
return DateTimeUtil.isoTimestamptzToNanos(timestampTzNano);
444-
} else {
445-
return DateTimeUtil.isoTimestampToNanos(value.textValue());
446-
}
447-
} else if (value.isIntegralNumber() && value.canConvertToLong()) {
448-
return value.longValue();
449-
} else {
450-
throw new IllegalArgumentException(
451-
String.format("Cannot parse default as a %s value: %s", type, value));
452-
}
453-
}
454417
}

core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ public static PlanTableScanResponse planTableScan(
660660
.fromSnapshotInclusive(request.startSnapshotId())
661661
.toSnapshot(request.endSnapshotId());
662662

663-
configuredScan = configureScan(incrementalScan, request, incrementalScan.schema());
663+
configuredScan = configureScan(incrementalScan, request);
664664
} else {
665665
// Regular table scan at a specific snapshot
666666
TableScan tableScan = table.newScan();
@@ -670,7 +670,7 @@ public static PlanTableScanResponse planTableScan(
670670
}
671671

672672
// Apply filters and projections using common method
673-
configuredScan = configureScan(tableScan, request, tableScan.schema());
673+
configuredScan = configureScan(tableScan, request);
674674
}
675675

676676
if (shouldPlanAsync.test(configuredScan)) {
@@ -776,18 +776,17 @@ static void clearPlanningState() {
776776
*
777777
* @param scan the scan to configure
778778
* @param request the plan table scan request containing filters and projections
779-
* @param schema the table schema to use for type-aware filter deserialization
780779
* @param <T> the specific scan type (TableScan, IncrementalAppendScan, etc.)
781780
* @return the configured scan with filters and projections applied
782781
*/
783782
private static <T extends Scan<T, FileScanTask, ?>> T configureScan(
784-
T scan, PlanTableScanRequest request, Schema schema) {
783+
T scan, PlanTableScanRequest request) {
785784
T configuredScan = scan;
786785

787786
if (request.select() != null) {
788787
configuredScan = configuredScan.select(request.select());
789788
}
790-
Expression filter = request.filter(schema);
789+
Expression filter = request.filter(scan.schema());
791790
if (filter != null) {
792791
configuredScan = configuredScan.filter(filter);
793792
}

0 commit comments

Comments
 (0)