Page MenuHomePhabricator

Do a better job of handling spec errors during schema adjustment
ClosedPublic

Authored by epriestley on Nov 1 2014, 5:02 PM.
Tags
None
Referenced Files
F14066076: D10771.diff
Tue, Nov 19, 8:04 AM
F14040695: D10771.diff
Mon, Nov 11, 12:29 PM
F14017616: D10771.id25872.diff
Mon, Nov 4, 7:51 PM
F14010733: D10771.diff
Thu, Oct 31, 12:11 PM
F14010687: D10771.id25872.diff
Thu, Oct 31, 11:41 AM
F14010680: D10771.id.diff
Thu, Oct 31, 11:36 AM
F14010646: D10771.id25856.diff
Thu, Oct 31, 11:29 AM
F13999786: D10771.diff
Thu, Oct 24, 5:13 PM
Subscribers

Details

Summary

Ref T1191. Currently if a developer forgot to specify a column type, storage adjust aborts explosively mid-stream. Instead:

  • Make this a formal error with an unambiugous name/description instead of something you sort of infer by seeing "<unknown>".
  • Make this error prevent generation of adjustment warnings, so we don't try to ALTER TABLE t CHANGE COLUMN c <unknown>, which is nonsense.
  • When schemata errors exist, surface them prominiently in storage adjust.

Overall:

  • Once storage upgrade runs storage adjust automatically (soon), this will make it relatively difficult to miss these errors.
  • Letting these errors slip through no longer escalates into a more severe issue.
Test Plan

Commented out the recent mailKey spec and ran storage adjust:

$ ./bin/storage adjust --force 
Verifying database schemata...
Found no adjustments for schemata.

Target                                            Error                      
phabricator2_phriction.phriction_document.mailKey Column Has No Specification

 SCHEMATA ERRORS 

The schemata have serious errors (detailed above) which the adjustment
workflow can not fix.

If you are not developing Phabricator itself, report this issue to the
upstream.

If you are developing Phabricator, these errors usually indicate that your
schema specifications do not agree with the schemata your code actually
builds.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Do a better job of handling spec errors during schema adjustment.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Nov 3 2014, 8:20 PM
This revision was automatically updated to reflect the committed changes.