From 85b4a31e2a05bac763f72ba8a129887e0772c0b3 Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Tue, 20 Mar 2018 11:12:18 -0700 Subject: [PATCH 1/2] scorch zap getField() which panics if the field is unknown --- index/scorch/segment/zap/new.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/index/scorch/segment/zap/new.go b/index/scorch/segment/zap/new.go index dd2740fb..68827f0b 100644 --- a/index/scorch/segment/zap/new.go +++ b/index/scorch/segment/zap/new.go @@ -17,6 +17,7 @@ package zap import ( "bytes" "encoding/binary" + "fmt" "math" "sort" "sync" @@ -288,6 +289,16 @@ func (s *interim) getOrDefineField(fieldName string) int { return int(fieldIDPlus1 - 1) } +// the fieldName must be for a known field +func (s *interim) getField(fieldName string) int { + fieldIDPlus1, exists := s.FieldsMap[fieldName] + if !exists || fieldIDPlus1 <= 0 { + panic(fmt.Sprintf("getField saw unknown fieldName: %s, fieldsMap: %#v", + fieldName, s.FieldsMap)) + } + return int(fieldIDPlus1 - 1) +} + // fill Dicts and DictKeys from analysis results func (s *interim) prepareDicts() { var pidNext int @@ -328,14 +339,14 @@ func (s *interim) prepareDicts() { for _, result := range s.results { // walk each composite field for _, field := range result.Document.CompositeFields { - fieldID := uint16(s.getOrDefineField(field.Name())) + fieldID := uint16(s.getField(field.Name())) _, tf := field.Analyze() visitField(fieldID, tf) } // walk each field for i, field := range result.Document.Fields { - fieldID := uint16(s.getOrDefineField(field.Name())) + fieldID := uint16(s.getField(field.Name())) tf := result.Analyzed[i] visitField(fieldID, tf) } @@ -439,14 +450,14 @@ func (s *interim) processDocument(docNum uint64, // walk each composite field for _, field := range result.Document.CompositeFields { - fieldID := uint16(s.getOrDefineField(field.Name())) + fieldID := uint16(s.getField(field.Name())) ln, tf := field.Analyze() visitField(fieldID, field.Name(), ln, tf) } // walk each field for i, field := range result.Document.Fields { - fieldID := uint16(s.getOrDefineField(field.Name())) + fieldID := uint16(s.getField(field.Name())) ln := result.Length[i] tf := result.Analyzed[i] visitField(fieldID, field.Name(), ln, tf) @@ -477,7 +488,7 @@ func (s *interim) processDocument(docNum uint64, for _, loc := range tf.Locations { var locf = uint16(fieldID) if loc.Field != "" { - locf = uint16(s.getOrDefineField(loc.Field)) + locf = uint16(s.getField(loc.Field)) } var arrayposs []uint64 if len(loc.ArrayPositions) > 0 { @@ -517,7 +528,7 @@ func (s *interim) writeStoredFields() ( } for _, field := range result.Document.Fields { - fieldID := uint16(s.getOrDefineField(field.Name())) + fieldID := uint16(s.getField(field.Name())) opts := field.Options() From 2f4d3d858751a019650379443709d700454149a7 Mon Sep 17 00:00:00 2001 From: Steve Yen Date: Tue, 20 Mar 2018 11:17:46 -0700 Subject: [PATCH 2/2] scorch zap panic if mergeFields() sees unsorted fields mergeFields depends on the fields from the various segments being sorted for the fieldsSame comparison to work. Of note, the 'fieldi > 1' guard skips the 0th field, which should always be the '_id' field. --- index/scorch/segment/zap/merge.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/index/scorch/segment/zap/merge.go b/index/scorch/segment/zap/merge.go index 1da5e526..622cf8cc 100644 --- a/index/scorch/segment/zap/merge.go +++ b/index/scorch/segment/zap/merge.go @@ -755,6 +755,10 @@ func mergeFields(segments []*SegmentBase) (bool, []string) { if len(segment0Fields) != len(fields) || segment0Fields[fieldi] != field { fieldsSame = false } + + if fieldi > 1 && field <= fields[fieldi-1] { + panic(fmt.Sprintf("mergeFields on unsorted fields: %#v", fields)) + } } }