Repository: ariga/atlas
Pull request #314 (2021-12-15 14:56:12)

sql/internal/specutil: support omitting right-most zero-value optional param
this fixes a bug where a typespec has an optional (non-required) type attribute, and the resulting DDL representation was too verbose. For instance, in some dialects both `varchar` and `varchar(20)` are valid. In these cases the `size` attribute is optional. In these cases if the size was not specified it would be rendered as `varchar(0)`. 

This PR adds a fix where the right-most optional args that have the zero value are omitted. so default:
`varchar(0)` => `varchar`, `x(10,0)` => `x(10)`, `x(0,10)` => remains `x(0,10)`


Patch:
diff --git a/sql/internal/specutil/types.go b/sql/internal/specutil/types.go
index ae14e541..5f29ecfa 100644
--- a/sql/internal/specutil/types.go
+++ b/sql/internal/specutil/types.go
@@ -135,7 +135,9 @@ func (r *TypeRegistry) Convert(typ schema.Type) (*schemaspec.Type, error) {
 	if !ok {
 		return nil, fmt.Errorf("specutil: type %q not found in registry", s.T)
 	}
-	for _, attr := range typeSpec.Attributes {
+	// Iterate the attributes in reverse order, so we can skip zero value and optional attrs.
+	for i := len(typeSpec.Attributes) - 1; i >= 0; i-- {
+		attr := typeSpec.Attributes[i]
 		n := inflect.Camelize(attr.Name)
 		field := rv.FieldByName(n)
 		if !field.IsValid() {
@@ -146,11 +148,19 @@ func (r *TypeRegistry) Convert(typ schema.Type) (*schemaspec.Type, error) {
 		}
 		switch attr.Kind {
 		case reflect.Int:
-			i := strconv.Itoa(int(field.Int()))
-			s.Attrs = append(s.Attrs, LitAttr(attr.Name, i))
+			v := int(field.Int())
+			if v == 0 && len(s.Attrs) == 0 {
+				break
+			}
+			i := strconv.Itoa(v)
+			s.Attrs = append([]*schemaspec.Attr{LitAttr(attr.Name, i)}, s.Attrs...)
 		case reflect.Bool:
-			b := strconv.FormatBool(field.Bool())
-			s.Attrs = append(s.Attrs, LitAttr(attr.Name, b))
+			v := field.Bool()
+			if !v && len(s.Attrs) == 0 {
+				break
+			}
+			b := strconv.FormatBool(v)
+			s.Attrs = append([]*schemaspec.Attr{LitAttr(attr.Name, b)}, s.Attrs...)
 		case reflect.Slice:
 			lits := make([]string, 0, field.Len())
 			for i := 0; i < field.Len(); i++ {
@@ -160,7 +170,7 @@ func (r *TypeRegistry) Convert(typ schema.Type) (*schemaspec.Type, error) {
 				}
 				lits = append(lits, strconv.Quote(fi.String()))
 			}
-			s.Attrs = append(s.Attrs, ListAttr(attr.Name, lits...))
+			s.Attrs = append([]*schemaspec.Attr{ListAttr(attr.Name, lits...)}, s.Attrs...)
 		default:
 			return nil, fmt.Errorf("specutil: unsupported attr kind %s for attribute %q of %q", attr.Kind, attr.Name, typeSpec.Name)
 		}


Test patch:
diff --git a/sql/internal/specutil/types_test.go b/sql/internal/specutil/types_test.go
index 4ee551c4..dc51d555 100644
--- a/sql/internal/specutil/types_test.go
+++ b/sql/internal/specutil/types_test.go
@@ -110,12 +110,29 @@ func TestRegistryConvert(t *testing.T) {
 			expected: &schemaspec.Type{T: "int", Attrs: []*schemaspec.Attr{LitAttr("unsigned", "true")}},
 		},
 		{
-			typ: &schema.DecimalType{T: "decimal", Precision: 10, Scale: 2},
+			typ: &schema.DecimalType{T: "decimal", Precision: 10, Scale: 2}, // decimal(10,2)
 			expected: &schemaspec.Type{T: "decimal", Attrs: []*schemaspec.Attr{
 				LitAttr("precision", "10"),
 				LitAttr("scale", "2"),
 			}},
 		},
+		{
+			typ: &schema.DecimalType{T: "decimal", Precision: 10}, // decimal(10)
+			expected: &schemaspec.Type{T: "decimal", Attrs: []*schemaspec.Attr{
+				LitAttr("precision", "10"),
+			}},
+		},
+		{
+			typ: &schema.DecimalType{T: "decimal", Scale: 2}, // decimal(0,2)
+			expected: &schemaspec.Type{T: "decimal", Attrs: []*schemaspec.Attr{
+				LitAttr("precision", "0"),
+				LitAttr("scale", "2"),
+			}},
+		},
+		{
+			typ:      &schema.DecimalType{T: "decimal"}, // decimal
+			expected: &schemaspec.Type{T: "decimal"},
+		},
 		{
 			typ: &schema.EnumType{T: "enum", Values: []string{"on", "off"}},
 			expected: &schemaspec.Type{T: "enum", Attrs: []*schemaspec.Attr{