Re: TableMetadata.buildReplacement() adds old spec list

[OpenInx <op...@gmail.com>]

Hi
I guess we shouldn't do the change, because the immutable partition spec
list seems try to maintain all
the history specs, not only the latest version.  If we change here, then
would break the specs & specsById
definition in TableMetadata.

> This change is breaking
TestReplaceTransaction.testReplaceWithIncompatibleSchemaUpdate test...
What's the code change ? Could provide the patch so that we can have a test
under my host ?

Thanks.


On Sat, Jan 18, 2020 at 6:56 AM Yathindranath P <ya...@gmail.com>
wrote:

> Hi Ryan,
>
> I noticed, TableMetadata.buildReplacement() is adding old partition spec
> to the new TableMetadata.
>
>     ImmutableList.Builder<PartitionSpec> builder =
> ImmutableList.<PartitionSpec>builder()
>         .addAll(specs);
>     if (!specsById.containsKey(specId)) {
>       builder.add(freshSpec);
>     }
>
> I'm working expire metadata with ExpireSnapshots, as part of this I'm
> updating TestTableOperations.commit to write and read metadata json file.
>
> This change is breaking
> TestReplaceTransaction.testReplaceWithIncompatibleSchemaUpdate test case,
> because PartitionSpecParser.fromJson() is trying to read a column which is
> not present in the replaced schema.
>
> Should we change the logic to add old specs only if specId is not present
> in the specsById as below?
>
>     ImmutableList.Builder<PartitionSpec> builder =
> ImmutableList.<PartitionSpec>builder();
>     if (!specsById.containsKey(specId)) {
>       builder.add(freshSpec);
>     } else {
>         builder.addAll(specs);
>    }
>
> Thanks,
> Yathi
>