Skip to content

API: Use normalized JSON path to identify Variant fields - #12835

Merged
amogh-jahagirdar merged 1 commit into
apache:mainfrom
rdblue:variant-metrics-use-normalized-path
Apr 21, 2025
Merged

API: Use normalized JSON path to identify Variant fields#12835
amogh-jahagirdar merged 1 commit into
apache:mainfrom
rdblue:variant-metrics-use-normalized-path

Conversation

@rdblue

Copy link
Copy Markdown
Contributor

This updates the Parquet metrics conversion to use normalized JSON paths instead of joining field names using .. A normalized path is a more reliable representation that preserves the distinction between [a.b][c], [a][b][c] and [a.b.c].

This implements the changes from #12658. Currently, the spec does not cover how bounds should be stored for variants, but I think we are in agreement about using normalized paths, so I would like to get this in to avoid writing incorrect bounds in the 1.9.0 release.

@rdbluerdblue added this to the Iceberg 1.9.0 milestone Apr 17, 2025
@rdblue
rdblueforce-pushed the variant-metrics-use-normalized-path branch from 6ec3ae4 to 86308d4CompareApril 17, 2025 18:23
@rdblue
rdblueforce-pushed the variant-metrics-use-normalized-path branch from 86308d4 to fe675baCompareApril 17, 2025 18:26
return names;
}

public static String toNormalizedPath(Iterable<String> fields) {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This has to be public so that the Parquet conversion code can use it to translate a list of field names into a normalized path.

@aihuaxuaihuaxu 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.

I compared with rfc doc (https://datatracker.ietf.org/doc/rfc9535/) with unit tests.

@ajantha-bhat

Copy link
Copy Markdown
Member

Will cherry-pick this to the next RC once merged. cc: @nastra

@amogh-jahagirdar
amogh-jahagirdar self-requested a review April 18, 2025 22:21
@amogh-jahagirdar

Copy link
Copy Markdown
Contributor

thanks @rdblue and thank you @aihuaxu for the spec change for this and for reviewing! I'll go ahead and merge

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rdblue@ajantha-bhat@amogh-jahagirdar@aihuaxu