Skip to content

Use ID-based mapping for struct fields in Iceberg Parquet reader vol. 2#9124

Merged
findepi merged 5 commits intotrinodb:masterfrom
findepi:findepi/parquet-2
Sep 7, 2021
Merged

Use ID-based mapping for struct fields in Iceberg Parquet reader vol. 2#9124
findepi merged 5 commits intotrinodb:masterfrom
findepi:findepi/parquet-2

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Sep 6, 2021

Like #9104 but with splitting ParquetColumnIOConverter instead of refactoring it.

Fixes #8750

- cast after `instanceof` check
- convert use of `getTypeParameters` to direct use of type's attributes
- `Optional.get` -> `Optional.orElseThrow`
- rename `ParquetColumnIOConverter` to `HiveParquetColumnIOConverter`
- the new class `IcebergParquetColumnIOConverter` is a copy of
  `ParquetColumnIOConverter`, with only class name changed.
return null;
}

@Nullable
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not pretty, but consistent with other methods in this file. A change to Optional can be a follow up, if needed.

Copy link
Copy Markdown
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

LGTM

@findepi
Copy link
Copy Markdown
Member Author

findepi commented Sep 7, 2021

Conflicts with #8129, but @alexjo2144 mentioned to me he and @phd3 concluded this should go before dereference pushdown.

@findepi findepi merged commit 09dc0bf into trinodb:master Sep 7, 2021
@findepi findepi deleted the findepi/parquet-2 branch September 7, 2021 15:04
@findepi findepi added this to the 362 milestone Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Unexpected results when reading Iceberg Parquet table after nested field schema evolved

2 participants