Skip to content

ORC: Use ConstantReader for identity partition columns #1191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rdblue merged 3 commits into from
Jul 11, 2020
Merged

ORC: Use ConstantReader for identity partition columns #1191

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c55c16e
ORC: Use ConstantReader for identity partition columns
shardulm94 Jul 10, 2020
26cbff7
Do not project constant columns from ORC reader
shardulm94 Jul 10, 2020
1439094
Address PR comments
shardulm94 Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 23 additions & 18 deletions orc/src/main/java/org/apache/iceberg/orc/OrcValueReaders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.types.Types;
import org.apache.orc.storage.ql.exec.vector.BytesColumnVector;
import org.apache.orc.storage.ql.exec.vector.ColumnVector;
Expand Down Expand Up @@ -59,6 +58,10 @@ public static OrcValueReader<byte[]> bytes() {
return BytesReader.INSTANCE;
}

public static <C> OrcValueReader<C> constants(C constant) {
return new ConstantReader<>(constant);
}

private static class BooleanReader implements OrcValueReader<Boolean> {
static final BooleanReader INSTANCE = new BooleanReader();

Expand Down Expand Up @@ -136,31 +139,20 @@ public byte[] nonNullRead(ColumnVector vector, int row) {

public abstract static class StructReader<T> implements OrcValueReader<T> {
private final OrcValueReader<?>[] readers;
private final int[] positions;
private final Object[] constants;

protected StructReader(List<OrcValueReader<?>> readers) {
this.readers = readers.toArray(new OrcValueReader[0]);
this.positions = new int[0];
this.constants = new Object[0];
}

protected StructReader(List<OrcValueReader<?>> readers, Types.StructType struct, Map<Integer, ?> idToConstant) {
this.readers = readers.toArray(new OrcValueReader[0]);
List<Types.NestedField> fields = struct.fields();
List<Integer> positionList = Lists.newArrayListWithCapacity(fields.size());
List<Object> constantList = Lists.newArrayListWithCapacity(fields.size());
for (int pos = 0; pos < fields.size(); pos += 1) {
Types.NestedField field = fields.get(pos);
Object constant = idToConstant.get(field.fieldId());
if (constant != null) {
positionList.add(pos);
constantList.add(idToConstant.get(field.fieldId()));
if (idToConstant.containsKey(field.fieldId())) {
this.readers[pos] = constants(idToConstant.get(field.fieldId()));
}
}

this.positions = positionList.stream().mapToInt(Integer::intValue).toArray();
this.constants = constantList.toArray();
}

protected abstract T create();
Expand All @@ -181,12 +173,25 @@ private T readInternal(T struct, ColumnVector[] columnVectors, int row) {
for (int c = 0; c < readers.length; ++c) {
set(struct, c, reader(c).read(columnVectors[c], row));
}
return struct;
}
}

for (int i = 0; i < positions.length; i += 1) {
set(struct, positions[i], constants[i]);
}
private static class ConstantReader<C> implements OrcValueReader<C> {
private final C constant;

return struct;
private ConstantReader(C constant) {
this.constant = constant;
}

@Override
public C read(ColumnVector ignored, int ignoredRow) {
Copy link
Contributor

@rdblue rdblue Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ColumnVector still materialized? Is it possible to avoid reading that entirely?

Copy link
Contributor Author

@shardulm94 shardulm94 Jul 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I guess we can do that by not asking ORC to project these columns. Let me give it a try.

Copy link
Contributor

@rdblue rdblue Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's going to be a bigger time savings and that's what we do for Parquet. We just drop it from the projection we pass down to the format.

return constant;
}

@Override
public C nonNullRead(ColumnVector ignored, int ignoredRow) {
return constant;
}
}
}