Sorry, I didn't come back to this after I initially read it. I think it's fine to make this change because we can definitely have identity transform partition fields that don't match after a rename. If I remember correctly, the reason for not making this public was just to ensure partition field names didn't deviate needlessly from the schema names, but this is a reasonable use case for making it public.
On Thu, May 29, 2025 at 11:24 AM Russell Spitzer <russell.spit...@gmail.com> wrote: > I'm not seeing any strong feelings on this so I'm going to go ahead and > merge. If anyone else sees issues we can always address this in a follow up. > > On Wed, May 21, 2025 at 6:07 PM Steven Wu <stevenz...@gmail.com> wrote: > >> It seems that the PR has made two valid arguments to support to change of >> public scope >> * identity transform builder is the only one where targetName builder is >> not public >> * handle the partition column rename use case >> >> So it seems reasonable to me. >> >> >> On Wed, May 21, 2025 at 2:49 PM Russell Spitzer < >> russell.spit...@gmail.com> wrote: >> >>> Hi Y'all >>> >>> We've been considering making a change to the Identity Partition >>> Transform >>> builder. Unlikely all of the other builders, Identity doesn't allow you >>> to make >>> an Identity Transform with a name different from the column you are >>> transforming. >>> >>> We want to be able to construct in memory a TableMetadata object which >>> matches >>> an existing Table without going through deserialization of a Json object >>> and >>> this is one of the few places where we can't actually legally build the >>> metadata which >>> matches the Json on disk. Meaning if you have a Table whose source >>> column of an >>> identity was changed, it is impossible to build metadata from the >>> TableMetadata.Builder >>> which matches that object. >>> >>> I was wondering if anyone has feelings about making the constructor >>> public or if >>> anyone knows of any reasons why making this public could cause problems. >>> >>> https://github.com/apache/iceberg/issues/12943 >>> https://github.com/apache/iceberg/pull/12975 >>> >>> I was hypothesizing that this relates to Hive partition mapping but I >>> can't think >>> of another reason it might matter. >>> >>> Thanks for reading and I'm eager to hear anyones thoughts, >>> Russ >>> >>
