Skip to content

Parquet: Fix typo in BaseParquetReaders and BaseParquet…#15244

Open
HennesyChihiro wants to merge 1 commit intoapache:mainfrom
HennesyChihiro:fix-typo
Open

Parquet: Fix typo in BaseParquetReaders and BaseParquet…#15244
HennesyChihiro wants to merge 1 commit intoapache:mainfrom
HennesyChihiro:fix-typo

Conversation

@HennesyChihiro
Copy link
Contributor

Summary
This PR cleans up documentation, fixes typos, and corrects inconsistent error messages in the Parquet data package, specifically within BaseParquetReaders and BaseParquetWriter.

Changes
BaseParquetWriter: Fixed an error message in the visit(IntLogicalTypeAnnotation) method where it incorrectly used "read" instead of "write" when handling uint64.

BaseParquetReaders: Completed an unfinished comment regarding missing nested field IDs and fixed minor typos.

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

thanks for the PR! a couple of comments

public ParquetValueReader<?> struct(
Types.StructType expected, GroupType struct, List<ParquetValueReader<?>> fieldReaders) {
// the expected struct is ignored because nested fields are never found when the
// the expected struct is ignored because nested fields are never found when the ID is missing
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

i asked claude to write a better comment, heres what it came up with. thoughts?

      // Unlike ReadBuilder.struct(), this does not reorder fields to match the expected schema
      // or handle missing fields with defaults. Without field IDs in the Parquet schema, fields
      // cannot be matched by ID, so they are read in file order instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants