zhangbutao commented on code in PR #10449:
URL: https://github.com/apache/iceberg/pull/10449#discussion_r1629227339
##########
mr/src/test/java/org/apache/iceberg/mr/TestIcebergInputFormats.java:
##########
@@ -234,6 +234,8 @@ public void testProjection() throws Exception {
Schema projection = TypeUtil.select(SCHEMA, ImmutableSet.of(1));
builder.project(projection);
+ // project() is the first to be applied on scan, so the select() won't
take effect.
+ builder.select("id");
Review Comment:
@pvary I tried to add a better ut to verify this change, but i found it is a
little difficult.
I can not get the iceberg `scan` which is initialized in `IcebergInputFormat
`to check whether the `project()` or `select()` is applied or not.
So i think we can check this change like this:
1) It is obvious that we need to use the new `scan` after applying
`project()` or `select()`. Right?
```
@@ -125,11 +125,11 @@ public class IcebergInputFormat<T> extends
InputFormat<Void, T> {
}
String schemaStr = conf.get(InputFormatConfig.READ_SCHEMA);
if (schemaStr != null) {
- scan.project(SchemaParser.fromJson(schemaStr));
+ scan = scan.project(SchemaParser.fromJson(schemaStr));
}
String[] selectedColumns =
conf.getStrings(InputFormatConfig.SELECTED_COLUMNS);
if (selectedColumns != null) {
- scan.select(selectedColumns);
+ scan = scan.select(selectedColumns);
}
```
After using the new scan, we can get execption when running the test:
`java.lang.IllegalStateException: Cannot select columns when projection
schema is set`, the exception is the same as
https://github.com/apache/iceberg/pull/1353/files#diff-a94dee2a22a2890689a84529a978a769bdc3b084f35d79caf9608d49b2e1cc1b
```
@TestTemplate
public void testProjection() throws Exception {
helper.createTable();
List<Record> inputRecords = helper.generateRandomRecords(1, 0L);
helper.appendToTable(Row.of("2020-03-20", 0), inputRecords);
Schema projection = TypeUtil.select(SCHEMA, ImmutableSet.of(1));
builder.project(projection);
// project() is the first to be applied on scan, so the select() won't
take effect.
builder.select("id");
List<Record> outputRecords =
testInputFormat.create(builder.conf()).getRecords();
assertThat(outputRecords).hasSameSizeAs(inputRecords);
assertThat(outputRecords.get(0).struct()).isEqualTo(projection.asStruct());
}
```
2) After using the new scan, we **can not be specified at the same time**.
So this change is the final fix.
Do you have any other ideas to write the test?
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
