Skip to content

MINOR: Fix display of logicalTypeAnnotation for parquet cli - #3184

Merged
wgtmac merged 2 commits into
apache:masterfrom
pan3793:cli-ann
Apr 29, 2025
Merged

MINOR: Fix display of logicalTypeAnnotation for parquet cli#3184
wgtmac merged 2 commits into
apache:masterfrom
pan3793:cli-ann

Conversation

@pan3793

@pan3793pan3793 commented Apr 8, 2025

Copy link
Copy Markdown
Member

Rationale for this change

I found the partial output of parquet footer xxx.parquet is incorrect, for example,

"logicalTypeAnnotation" : { }

while the value is actually a DateLogicalTypeAnnotation

What changes are included in this PR?

Supply a custom JsonSerializer for LogicalTypeAnnotation

Are these changes tested?

Currently, there is no output verification for parquet-cli modules, I manually checked the output of org.apache.parquet.cli.commands.ShowFooterCommandTest

 }, {
"name" : "date_field",
"repetition" : "REQUIRED",
- "logicalTypeAnnotation" : { },+ "logicalTypeAnnotation" : "DATE",
"id" : null,
"primitive" : "INT32",
"length" : 0,
"decimalMeta" : null,
"columnOrder" : {
"columnOrderName" : "TYPE_DEFINED_ORDER"
}
} ],

Are there any user-facing changes?

Only affects the output of parquet-cli.

@pan3793

Copy link
Copy Markdown
MemberAuthor

cc @wgtmac@Fokko

@FokkoFokko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this @pan3793! 🙌 This makes sense to me. Would it be possible to throw in a test? Just to avoid that we break this in the future

@pan3793

Copy link
Copy Markdown
MemberAuthor

@Fokko it has non-trivial work to add test to parquet-cli module to verify the output of commands, see #3187

@pan3793

Copy link
Copy Markdown
MemberAuthor

Hi @Fokko, please take a look at #3187 and let me know if you still pursue UT coverage for this change, or just merge it.

@wgtmacwgtmac changed the title Fix display of logicalTypeAnnotation for parquet cliMINOR: Fix display of logicalTypeAnnotation for parquet cliApr 29, 2025
@wgtmac
wgtmac merged commit 32a484a into apache:masterApr 29, 2025
@wgtmacwgtmac added this to the 1.16.0 milestone Apr 29, 2025
Sign up for freeto 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.

3 participants

@pan3793@Fokko@wgtmac