remove mapstructure

I have tripped over mapstructure one too many times so this commit
removes it.  When/if the extra round-trip through JSON adds
measurable overhead to Zed query times, we can revisit this and
hopefully mapstructure will be more mature.  For now, I wanted to
add order.Which to proc.Sort and similar but mapstructure cannot
handle custom JSON unmarshalers so this caused unpack to fail
for apparently no reason.

See mitchellh/mapstructure#115

compiler/ast/dag/unpack.go

@@ -1,6 +1,7 @@
 package dag
 
 import (
+	"encoding/json"
 	"errors"
 
 	"github.com/brimdata/zed/compiler/ast/zed"
@@ -80,7 +81,7 @@ func UnpackJSON(buf []byte) (interface{}, error) {
 	if len(buf) == 0 {
 		return nil, nil
 	}
-	return unpacker.UnpackBytes(buf)
+	return unpacker.Unmarshal(buf)
 }
 
 // UnpackJSONAsOp transforms a JSON representation of an operator into an dag.Op.
@@ -97,7 +98,11 @@ func UnpackJSONAsOp(buf []byte) (Op, error) {
 }
 
 func UnpackMapAsOp(m interface{}) (Op, error) {
-	object, err := unpacker.UnpackMap(m)
+	b, err := json.Marshal(m)
+	if err != nil {
+		return nil, err
+	}
+	object, err := unpacker.Unmarshal(b)
 	if object == nil || err != nil {
 		return nil, err
 	}
@@ -109,7 +114,11 @@ func UnpackMapAsOp(m interface{}) (Op, error) {
 }
 
 func UnpackMapAsExpr(m interface{}) (Expr, error) {
-	object, err := unpacker.UnpackMap(m)
+	b, err := json.Marshal(m)
+	if err != nil {
+		return nil, err
+	}
+	object, err := unpacker.Unmarshal(b)
 	if object == nil || err != nil {
 		return nil, err
 	}

compiler/ast/unpack.go

@@ -86,7 +86,7 @@ func UnpackJSON(buf []byte) (interface{}, error) {
 	if len(buf) == 0 {
 		return nil, nil
 	}
-	return unpacker.UnpackBytes(buf)
+	return unpacker.Unmarshal(buf)
 }
 
 // UnpackJSONAsProc transforms a JSON representation of a proc into an ast.Proc.
@@ -103,7 +103,11 @@ func UnpackJSONAsProc(buf []byte) (Proc, error) {
 }
 
 func UnpackMapAsProc(m interface{}) (Proc, error) {
-	object, err := unpacker.UnpackMap(m)
+	b, err := json.Marshal(m)
+	if err != nil {
+		return nil, err
+	}
+	object, err := unpacker.Unmarshal(b)
 	if object == nil || err != nil {
 		return nil, err
 	}
@@ -115,7 +119,11 @@ func UnpackMapAsProc(m interface{}) (Proc, error) {
 }
 
 func UnpackMapAsExpr(m interface{}) (Expr, error) {
-	object, err := unpacker.UnpackMap(m)
+	b, err := json.Marshal(m)
+	if err != nil {
+		return nil, err
+	}
+	object, err := unpacker.Unmarshal(b)
 	if object == nil || err != nil {
 		return nil, err
 	}

go.mod

@@ -19,7 +19,6 @@ require (
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/kr/text v0.2.0
 	github.com/mattn/go-isatty v0.0.12 // indirect
-	github.com/mitchellh/mapstructure v1.3.3
 	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
 	github.com/pbnjay/memory v0.0.0-20190104145345-974d429e7ae4
 	github.com/peterh/liner v1.1.0

go.sum

@@ -112,8 +112,6 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0j
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
-github.com/mitchellh/mapstructure v1.3.3 h1:SzB1nHZ2Xi+17FP0zVQBHIZqvwRN9408fJO8h+eeNA8=
-github.com/mitchellh/mapstructure v1.3.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=

pkg/unpack/reflector.go

@@ -4,8 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
-
-	"github.com/mitchellh/mapstructure"
 )
 
 var zero reflect.Value
@@ -69,19 +67,15 @@ func (r Reflector) get(unpackKey string, create bool) map[string]reflect.Type {
 	return types
 }
 
-func (r Reflector) Unpack(s string) (interface{}, error) {
-	return r.UnpackBytes([]byte(s))
+func (r Reflector) UnmarshalString(s string) (interface{}, error) {
+	return r.Unmarshal([]byte(s))
 }
 
-func (r Reflector) UnpackBytes(b []byte) (interface{}, error) {
-	var jsonMap interface{}
-	if err := json.Unmarshal(b, &jsonMap); err != nil {
+func (r Reflector) Unmarshal(b []byte) (interface{}, error) {
+	var m interface{}
+	if err := json.Unmarshal(b, &m); err != nil {
 		return nil, fmt.Errorf("unpacker error parsing JSON: %w", err)
 	}
-	return r.UnpackMap(jsonMap)
-}
-
-func (r Reflector) UnpackMap(m interface{}) (interface{}, error) {
 	object, ok := m.(map[string]interface{})
 	if !ok {
 		return nil, fmt.Errorf("cannot unpack non-object JSON value")
@@ -93,22 +87,10 @@ func (r Reflector) UnpackMap(m interface{}) (interface{}, error) {
 	if rv, ok := skeleton.(reflect.Value); ok {
 		skeleton = rv.Interface()
 	}
-	v := skeleton
-	if _, ok := v.(map[string]interface{}); ok {
-		// If the root record wasn't decoded to a struct ptr,
-		// we pass a pointer to mapstructure as it requires
-		// a pointer val.
-		v = &skeleton
-	}
-	c := &mapstructure.DecoderConfig{
-		TagName: "json",
-		Result:  v,
-	}
-	dec, err := mapstructure.NewDecoder(c)
-	if err != nil {
-		return nil, fmt.Errorf("unpack (mapstructure): %w", err)
+	if err := json.Unmarshal(b, &skeleton); err != nil {
+		return nil, err
 	}
-	return skeleton, dec.Decode(m)
+	return skeleton, err
 }
 
 func (r Reflector) lookup(object map[string]interface{}) (reflect.Value, error) {

pkg/unpack/unpack_test.go

@@ -51,7 +51,7 @@ func TestUnpackBinaryExpr(t *testing.T) {
 		Terminal{},
 		List{},
 	)
-	actual, err := reflector.Unpack(binaryExprJSON)
+	actual, err := reflector.UnmarshalString(binaryExprJSON)
 	require.NoError(t, err)
 	assert.Equal(t, binaryExprExpected, actual)
 }
@@ -80,7 +80,7 @@ func TestUnpackTypeTag(t *testing.T) {
 		BinaryExpr2{},
 		Terminal{},
 	)
-	actual, err := reflector.Unpack(typeTagJSON)
+	actual, err := reflector.UnmarshalString(typeTagJSON)
 	require.NoError(t, err)
 	assert.Equal(t, typeTagExpected, actual)
 }
@@ -119,7 +119,7 @@ func TestUnpackNested(t *testing.T) {
 		Terminal{},
 		List{},
 	)
-	actual, err := reflector.Unpack(nestedJSON)
+	actual, err := reflector.UnmarshalString(nestedJSON)
 	require.NoError(t, err)
 	assert.Equal(t, nestedExpected, actual)
 }
@@ -172,7 +172,7 @@ func TestUnpackEmbedded(t *testing.T) {
 		List{},
 		Embedded{},
 	)
-	actual, err := reflector.Unpack(embeddedJSON)
+	actual, err := reflector.UnmarshalString(embeddedJSON)
 	require.NoError(t, err)
 	assert.Equal(t, embeddedExpected, actual)
 }
@@ -197,7 +197,7 @@ func TestUnpackList(t *testing.T) {
 		Terminal{},
 		List{},
 	)
-	actual, err := reflector.Unpack(listJSON)
+	actual, err := reflector.UnmarshalString(listJSON)
 	require.NoError(t, err)
 	assert.Equal(t, listExpected, actual)
 }
@@ -242,7 +242,7 @@ func TestUnpackPairList(t *testing.T) {
 		Terminal{},
 		PairList{},
 	)
-	actual, err := reflector.Unpack(pairListJSON)
+	actual, err := reflector.UnmarshalString(pairListJSON)
 	require.NoError(t, err)
 	assert.Equal(t, pairListExpected, actual)
 }
@@ -321,7 +321,7 @@ func TestUnpackCut(t *testing.T) {
 		Identifier{},
 		Assignment{},
 	)
-	actual, err := reflector.Unpack(cutJSON)
+	actual, err := reflector.UnmarshalString(cutJSON)
 	require.NoError(t, err)
 	assert.Equal(t, cutExpected, actual)
 }
@@ -352,7 +352,7 @@ func TestUnpackSkip(t *testing.T) {
 		BinaryExpr3{},
 		Terminal{},
 	)
-	actual, err := reflector.Unpack(skipJSON)
+	actual, err := reflector.UnmarshalString(skipJSON)
 	require.NoError(t, err)
 	assert.Equal(t, skipExpected, actual)
 }

zio/zjsonio/writer.go

@@ -26,7 +26,16 @@ func unmarshal(b []byte) (*Object, error) {
 	}
 	var types []zed.Type
 	for _, t := range template.Types {
-		object, err := unpacker.UnpackMap(t)
+		// We should enhance the unpacker to take the template struct
+		// here so we don't have to do this song and dance.  But not
+		// a big deal because we only do it for inbound zjson (which is
+		// not performance critical and only for typedefs which are
+		// typically infrequent.)  See issue #2702.
+		b, err := json.Marshal(t)
+		if err != nil {
+			return nil, err
+		}
+		object, err := unpacker.Unmarshal(b)
 		if object == nil || err != nil {
 			return nil, err
 		}