Add backwards-compatibility checking for AlgebraicTypeLayouts used in system tables#1737
Add backwards-compatibility checking for AlgebraicTypeLayouts used in system tables#1737
Conversation
| && self_elt.offset == other_elt.offset | ||
| }) | ||
| } | ||
| (Sum(self_sum), Sum(other_sum)) => { |
There was a problem hiding this comment.
very not sure about this bit
system tables. Add comment More tidying One more comment
kazimuth
force-pushed
the
jgilles/bsatn_subtype
branch
from
7892d87 to
485350b
Compare
| ], | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar calls of this could benefit by a helper like:
impl HasOnePointOhLayout for StTableRow {
fn get_original_layout() -> AlgebraicTypeLayout {
Product(ProductTypeLayout {
layout: Layout { size: 24, align: 4 },
elements: boxed![
element_layout(0, Primitive(U32), "table_id"),
element_layout(4, VarLen(String), "table_name"),
element_layout(8, VarLen(String), "table_type"),
element_layout(12, VarLen(String), "table_access"),
element_layout(16, ..., "table_primary_key"),
],
})
}
}
gefjon
left a comment
gefjon
left a comment
There was a problem hiding this comment.
I have not carefully checked the layouts you declare in test_one_point_oh_layouts.rs, but I agree with your subtyping logic. This needs a rebase/merge with master to get the internal checks job passing, but then could merge whenever.
| /// - `self.align() >= other.align()` (in other words, `other.align()` divides `self.align()`) | ||
| /// | ||
| /// - If `self` is a product type, then: | ||
| /// - `other` must also be a product type. |
There was a problem hiding this comment.
This is a stronger requirement than we actually need - one-element products are representationally equivalent to their contents. But I don't see any downside to imposing this restriction artificially.
| // We don't need to check variants that are not in `self`. | ||
| // Their alignment requirements were already implicitly checked by the size and alignment checks at the entry | ||
| // of this function. |
Signed-off-by: james gilles <jameshgilles@gmail.com>
Signed-off-by: james gilles <jameshgilles@gmail.com>
|
|
|
Merged I also ported the old system-table 1.0 layout compatibility coverage onto the current |
7 participants
Description of Changes
Add backwards-compatibility checking for AlgebraicTypeLayouts used in system tables. This required implementing backwards-compatibility validation logic for BSATN and BFLATN. I still need to add an actual serialization test for BFLATN but I'm still figuring out the plumbing there.
This doesn't need to happen until post-1.0.
API and ABI breaking changes
None
Expected complexity level and risk
0
Testing
Added proptests and some manual tests, but it isn't airtight yet.