iceberg icon indicating copy to clipboard operation
iceberg copied to clipboard

Improve backward compatibility tests for spec changes introduced in all table versions

Open yyanyy opened this issue 3 years ago • 2 comments

Stem from this comment: https://github.com/apache/iceberg/pull/2521#issuecomment-828793923

We had a NPE bug (#2405) caused by reading a newly introduced field as boolean, which assigns null to this boolean field when reading old tables. While Iceberg has backward compatibility tests to test reading V1 tables, this field is introduced for all specs, and thus when creating V1 table for testing, this field will still be populated. NPE only occurs in a wider setting - during table commits (fixed in #2495) or reading manifest table (fixed in #2521).

I think to prevent this from happening in future, we may need to introduce a mechanism that creates table not through Iceberg code itself, so that the new fields introduced to the spec can continue to be null. The comment link above describes a proposal to achieve it but there might be better approaches.

yyanyy avatar Apr 28 '21 21:04 yyanyy

A bit late to the discussion, and didnt look very closely, but I wonder if we can make Testwriters for the various metadata that writes null for all optional fields, and then try to deserialize it with the regular readers, to test all newly-fields at once. @RussellSpitzer , @yyanyy

szehon-ho avatar Jun 13 '22 22:06 szehon-ho

Was chatting with @dramaticlly about it, cc if you want to look at it.

szehon-ho avatar Aug 08 '22 22:08 szehon-ho

This issue has been automatically marked as stale because it has been open for 180 days with no activity. It will be closed in next 14 days if no further activity occurs. To permanently prevent this issue from being considered stale, add the label 'not-stale', but commenting on the issue is preferred when possible.

github-actions[bot] avatar Apr 16 '24 00:04 github-actions[bot]