Improve error reporting for invalid column-type-changing automigrations

[gefjon]

Description of Changes

Title. When validating column type changes in Table::change_columns_to and comparing layouts in RowTypeLayout::is_compatible_with, return structured error objects which describe the mismatch.

I made this change while debugging the issue that led to #3203 , where change_columns_to returned an error during replay of an automigration which had succeeded the first time. The original error contained enough information to debug, but it was presented in a noisy and unhelpful way, so I wrote this patch to get better diagnostics.

Per @Centril 's comments, these errors are for internal assertions, not user-facing error reporting, so we value fail-fast rather than error hygiene and choose not to use the ErrorStream combinator.

Also note that I sprinkled Boxes around some large-ish error types to quiet https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err .

API and ABI breaking changes

N/a

Expected complexity level and risk

Testing

Manual testing with BitCraft. As this is purely altering the error reporting for non-user-facing assertions, I don't believe any further testing is necessary.

[Centril]

So the structured errors already exist in ensure_old_ty_upgradable_to_new and they use ErrorStream.
The intent of the checks in ensure_old_ty_upgradable_to_new is as a soundness check, in case there's a bug in auto_migrate_table, rather than to be user facing. So I don't think ErrorStream is necessary there, but I do see some value in getting better errors directly from the datastore when debugging.

[gefjon]

So the structured errors already exist in ensure_old_ty_upgradable_to_new and they use ErrorStream. The intent of the checks in ensure_old_ty_upgradable_to_new is as a soundness check, in case there's a bug in auto_migrate_table, rather than to be user facing. So I don't think ErrorStream is necessary there, but I do see some value in getting better errors directly from the datastore when debugging.

In that case I'm happy to merge without worrying about using ErrorStream or getting too hung up about testing, though I do intend to leave the TODO comment in place.

[Centril]

[...] though I do intend to leave the TODO comment in place.

Not sure what the value of those TODOs are. I don't think they should be implemented, as all of the datastore is fail-fast and doesn't gather errors. If we want to change that I think we should do it as part of a larger plan.

[gefjon]

I don't think they should be implemented, as all of the datastore is fail-fast and doesn't gather errors.

Mm, fair. I'll remove.