Skip to content
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.

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

[Baseline] Apply baseline to iceberg-orc #158 #211

Merged
merged 4 commits into from
Jun 12, 2019

Conversation

rdsr
Copy link
Contributor

@rdsr rdsr commented Jun 8, 2019

Apply baseline to iceberg-orc #158

@rdsr rdsr changed the title Apply baseline to iceberg-orc #158 [Baseline] Apply baseline to iceberg-orc #158 Jun 11, 2019
return this;
}

public ReadBuilder schema(org.apache.iceberg.Schema schema) {
this.schema = schema;
public ReadBuilder schema(org.apache.iceberg.Schema projectedSchema) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: should probably be "projectSchema" not "projected". It doesn't make sense to use past tense; this is configuring a read that hasn't happened yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Function<TypeDescription, OrcValueWriter<?>> createWriterFunc) {
private static <D> OrcValueWriter<D> newOrcValueWriter(
TypeDescription schema,
Function<TypeDescription, OrcValueWriter<?>> createWriterFunc) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should have a newline. These should fit on one line, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! I'll update

@rdblue
Copy link
Contributor

rdblue commented Jun 11, 2019

I'd merge this since there are only nits, but it conflicts with the baseline patch I just merged. Could you rebase and update?

@rdblue rdblue merged commit d6f907f into apache:master Jun 12, 2019
deniskuzZ added a commit to deniskuzZ/iceberg that referenced this pull request May 8, 2024
(cherry picked from commit a41412bab8abb35be7f58a83ea2c6da4e5fc1880)
Change-Id: I5eea21f01f12a8b5c27e2595717427ad9284064d

fix: added github workflow yml (apache#209)

(cherry picked from commit fe20aaeb356c214d4590c691846e879c5ec7b6ea)

added github workflow yml (apache#206)

(cherry picked from commit 1fea75cf9e56b25942662092ffdc3cc2b425cc3d)
Change-Id: If1b1e404b10c9507e417166453a7eaa2aef98cf9
(cherry picked from commit 153c7d1a15597b5c0577641a517f9aebad17c912)
(cherry picked from commit 6a8a15520a0f045702a9f50aa93e52f34d1d3bb0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants