Repository: blevesearch/bleve
Pull request #1878 (2023-09-06 07:11:40)

MB-58388: Use custom datetime parsers in date range facet queries
# Jira

[MB-58388](https://issues.couchbase.com/browse/MB-58388)

## Description
 
Related to Date range facet queries:

- The "start" and "end" fields of the Facet Result struct are now formatted with the layout that successfully parsed them in the Facet Request struct. Earlier we always used RFC3339Nano by default which led to disparity between the corresponding fields in the facet request and result. 
   Ex - Facet Request Date Range:

          { 
              start : "2014-09-10"
              end : "2015-08-11"
          }

Previously the Facet result had 

          { 
              start : "2014-09-10T00:00:00Z"
              end : "2015-08-11T00:00:00Z"
              Count: 100
          }

Now it has 

          { 
              start : "2014-09-10"
              end : "2015-08-11"
              Count: 100
          }
- Allow a custom date time parser to be added to the date range struct of a facet request, that will override the default date time parser (dateTimeOptional) while parsing the start and end date strings.
- Add error message when the date time parser fails to parse the date string. Earlier if the date time parser failed to parse the start and end strings of the date range in the facet request, no error was thrown and the facet request would be for start and end being equal to 0 time, so facet result was always nil.
- Fix marshalling error for dateRangeQuery struct. If the query had no end / endString, which is the case when upper bound is unspecified, the marshalled JSON always had the 0 time (time.Time{}) as the value for end, now it is omitted from the marshalled JSON. 


Patch:
diff --git a/index_impl.go b/index_impl.go
index 592b3c69..6c47872d 100644
--- a/index_impl.go
+++ b/index_impl.go
@@ -530,10 +530,23 @@ func (i *indexImpl) SearchInContext(ctx context.Context, req *SearchRequest) (sr
 			} else if facetRequest.DateTimeRanges != nil {
 				// build date range facet
 				facetBuilder := facet.NewDateTimeFacetBuilder(facetRequest.Field, facetRequest.Size)
-				dateTimeParser := i.m.DateTimeParserNamed("")
 				for _, dr := range facetRequest.DateTimeRanges {
-					start, end := dr.ParseDates(dateTimeParser)
-					facetBuilder.AddRange(dr.Name, start, end)
+					dateTimeParserName := defaultDateTimeParser
+					if dr.DateTimeParser != "" {
+						dateTimeParserName = dr.DateTimeParser
+					}
+					dateTimeParser := i.m.DateTimeParserNamed(dateTimeParserName)
+					if dateTimeParser == nil {
+						return nil, fmt.Errorf("no date time parser named `%s` registered", dateTimeParserName)
+					}
+					start, end, startLayout, endLayout, err := dr.ParseDates(dateTimeParser)
+					if err != nil {
+						return nil, fmt.Errorf("ParseDates err: %v, using date time parser named %s", err, dateTimeParserName)
+					}
+					if start.IsZero() && end.IsZero() {
+						return nil, fmt.Errorf("date range query must specify either start, end or both for date range name '%s'", dr.Name)
+					}
+					facetBuilder.AddRange(dr.Name, start, end, startLayout, endLayout)
 				}
 				facetsBuilder.Add(facetName, facetBuilder)
 			} else {
diff --git a/search.go b/search.go
index a0fb024f..00e0a800 100644
--- a/search.go
+++ b/search.go
@@ -46,36 +46,44 @@ func init() {
 }
 
 type dateTimeRange struct {
-	Name        string    `json:"name,omitempty"`
-	Start       time.Time `json:"start,omitempty"`
-	End         time.Time `json:"end,omitempty"`
-	startString *string
-	endString   *string
+	Name           string    `json:"name,omitempty"`
+	Start          time.Time `json:"start,omitempty"`
+	End            time.Time `json:"end,omitempty"`
+	DateTimeParser string    `json:"datetime_parser,omitempty"`
+	startString    *string
+	endString      *string
 }
 
-func (dr *dateTimeRange) ParseDates(dateTimeParser analysis.DateTimeParser) (start, end time.Time) {
+func (dr *dateTimeRange) ParseDates(dateTimeParser analysis.DateTimeParser) (start, end time.Time, startLayout, endLayout string, err error) {
 	start = dr.Start
+	startLayout = time.RFC3339Nano
 	if dr.Start.IsZero() && dr.startString != nil {
-		s, _, err := dateTimeParser.ParseDateTime(*dr.startString)
-		if err == nil {
-			start = s
+		s, layout, parseError := dateTimeParser.ParseDateTime(*dr.startString)
+		if parseError != nil {
+			return start, end, startLayout, endLayout, fmt.Errorf("error parsing start date '%s' for date range name '%s': %v", *dr.startString, dr.Name, parseError)
 		}
+		start = s
+		startLayout = layout
 	}
 	end = dr.End
+	endLayout = time.RFC3339Nano
 	if dr.End.IsZero() && dr.endString != nil {
-		e, _, err := dateTimeParser.ParseDateTime(*dr.endString)
-		if err == nil {
-			end = e
+		e, layout, parseError := dateTimeParser.ParseDateTime(*dr.endString)
+		if parseError != nil {
+			return start, end, startLayout, endLayout, fmt.Errorf("error parsing end date '%s' for date range name '%s': %v", *dr.endString, dr.Name, parseError)
 		}
+		end = e
+		endLayout = layout
 	}
-	return start, end
+	return start, end, startLayout, endLayout, err
 }
 
 func (dr *dateTimeRange) UnmarshalJSON(input []byte) error {
 	var temp struct {
-		Name  string  `json:"name,omitempty"`
-		Start *string `json:"start,omitempty"`
-		End   *string `json:"end,omitempty"`
+		Name           string  `json:"name,omitempty"`
+		Start          *string `json:"start,omitempty"`
+		End            *string `json:"end,omitempty"`
+		DateTimeParser string  `json:"datetime_parser,omitempty"`
 	}
 
 	if err := json.Unmarshal(input, &temp); err != nil {
@@ -89,22 +97,33 @@ func (dr *dateTimeRange) UnmarshalJSON(input []byte) error {
 	if temp.End != nil {
 		dr.endString = temp.End
 	}
+	if temp.DateTimeParser != "" {
+		dr.DateTimeParser = temp.DateTimeParser
+	}
 
 	return nil
 }
 
 func (dr *dateTimeRange) MarshalJSON() ([]byte, error) {
 	rv := map[string]interface{}{
-		"name":  dr.Name,
-		"start": dr.Start,
-		"end":   dr.End,
+		"name": dr.Name,
 	}
-	if dr.Start.IsZero() && dr.startString != nil {
+
+	if !dr.Start.IsZero() {
+		rv["start"] = dr.Start
+	} else if dr.startString != nil {
 		rv["start"] = dr.startString
 	}
-	if dr.End.IsZero() && dr.endString != nil {
+
+	if !dr.End.IsZero() {
+		rv["end"] = dr.End
+	} else if dr.endString != nil {
 		rv["end"] = dr.endString
 	}
+
+	if dr.DateTimeParser != "" {
+		rv["datetime_parser"] = dr.DateTimeParser
+	}
 	return json.Marshal(rv)
 }
 
@@ -138,7 +157,7 @@ func (fr *FacetRequest) Validate() error {
 	nrCount := len(fr.NumericRanges)
 	drCount := len(fr.DateTimeRanges)
 	if nrCount > 0 && drCount > 0 {
-		return fmt.Errorf("facet can only conain numeric ranges or date ranges, not both")
+		return fmt.Errorf("facet can only contain numeric ranges or date ranges, not both")
 	}
 
 	if nrCount > 0 {
@@ -164,9 +183,16 @@ func (fr *FacetRequest) Validate() error {
 				return fmt.Errorf("date ranges contains duplicate name '%s'", dr.Name)
 			}
 			drNames[dr.Name] = struct{}{}
-			start, end := dr.ParseDates(dateTimeParser)
-			if start.IsZero() && end.IsZero() {
-				return fmt.Errorf("date range query must specify either start, end or both for range name '%s'", dr.Name)
+			if dr.DateTimeParser == "" {
+				// cannot parse the date range dates as the defaultDateTimeParser is overridden
+				// so perform this validation at query time
+				start, end, _, _, err := dr.ParseDates(dateTimeParser)
+				if err != nil {
+					return fmt.Errorf("ParseDates err: %v, using date time parser named %s", err, defaultDateTimeParser)
+				}
+				if start.IsZero() && end.IsZero() {
+					return fmt.Errorf("date range query must specify either start, end or both for range name '%s'", dr.Name)
+				}
 			}
 		}
 	}
@@ -186,7 +212,7 @@ func (fr *FacetRequest) AddDateTimeRange(name string, start, end time.Time) {
 }
 
 // AddDateTimeRangeString adds a bucket to a field
-// containing date values.
+// containing date values. Uses defaultDateTimeParser to parse the date strings.
 func (fr *FacetRequest) AddDateTimeRangeString(name string, start, end *string) {
 	if fr.DateTimeRanges == nil {
 		fr.DateTimeRanges = make([]*dateTimeRange, 0, 1)
@@ -195,6 +221,17 @@ func (fr *FacetRequest) AddDateTimeRangeString(name string, start, end *string)
 		&dateTimeRange{Name: name, startString: start, endString: end})
 }
 
+// AddDateTimeRangeString adds a bucket to a field
+// containing date values. Uses the specified parser to parse the date strings.
+// provided the parser is registered in the index mapping.
+func (fr *FacetRequest) AddDateTimeRangeStringWithParser(name string, start, end *string, parser string) {
+	if fr.DateTimeRanges == nil {
+		fr.DateTimeRanges = make([]*dateTimeRange, 0, 1)
+	}
+	fr.DateTimeRanges = append(fr.DateTimeRanges,
+		&dateTimeRange{Name: name, startString: start, endString: end, DateTimeParser: parser})
+}
+
 // AddNumericRange adds a bucket to a field
 // containing numeric values.  Documents with a
 // numeric value falling into this range are
diff --git a/search/facet/facet_builder_datetime.go b/search/facet/facet_builder_datetime.go
index ff5167f2..c272396b 100644
--- a/search/facet/facet_builder_datetime.go
+++ b/search/facet/facet_builder_datetime.go
@@ -17,6 +17,7 @@ package facet
 import (
 	"reflect"
 	"sort"
+	"strconv"
 	"time"
 
 	"github.com/blevesearch/bleve/v2/numeric"
@@ -35,8 +36,10 @@ func init() {
 }
 
 type dateTimeRange struct {
-	start time.Time
-	end   time.Time
+	start       time.Time
+	end         time.Time
+	startLayout string
+	endLayout   string
 }
 
 type DateTimeFacetBuilder struct {
@@ -75,10 +78,12 @@ func (fb *DateTimeFacetBuilder) Size() int {
 	return sizeInBytes
 }
 
-func (fb *DateTimeFacetBuilder) AddRange(name string, start, end time.Time) {
+func (fb *DateTimeFacetBuilder) AddRange(name string, start, end time.Time, startLayout string, endLayout string) {
 	r := dateTimeRange{
-		start: start,
-		end:   end,
+		start:       start,
+		end:         end,
+		startLayout: startLayout,
+		endLayout:   endLayout,
 	}
 	fb.ranges[name] = &r
 }
@@ -134,11 +139,23 @@ func (fb *DateTimeFacetBuilder) Result() *search.FacetResult {
 			Count: count,
 		}
 		if !dateRange.start.IsZero() {
-			start := dateRange.start.Format(time.RFC3339Nano)
+			var start string
+			if dateRange.startLayout == "" {
+				// layout not set probably means it is probably a timestamp
+				start = strconv.FormatInt(dateRange.start.UnixNano(), 10)
+			} else {
+				start = dateRange.start.Format(dateRange.startLayout)
+			}
 			tf.Start = &start
 		}
 		if !dateRange.end.IsZero() {
-			end := dateRange.end.Format(time.RFC3339Nano)
+			var end string
+			if dateRange.endLayout == "" {
+				// layout not set probably means it is probably a timestamp
+				end = strconv.FormatInt(dateRange.end.UnixNano(), 10)
+			} else {
+				end = dateRange.end.Format(dateRange.endLayout)
+			}
 			tf.End = &end
 		}
 		rv.DateRanges = append(rv.DateRanges, tf)


Test patch:
diff --git a/search_test.go b/search_test.go
index 6ddb861a..7f1ef0b8 100644
--- a/search_test.go
+++ b/search_test.go
@@ -282,7 +282,7 @@ func TestUnmarshalingSearchResult(t *testing.T) {
 func TestFacetNumericDateRangeRequests(t *testing.T) {
 	var drMissingErr = fmt.Errorf("date range query must specify either start, end or both for range name 'testName'")
 	var nrMissingErr = fmt.Errorf("numeric range query must specify either min, max or both for range name 'testName'")
-	var drNrErr = fmt.Errorf("facet can only conain numeric ranges or date ranges, not both")
+	var drNrErr = fmt.Errorf("facet can only contain numeric ranges or date ranges, not both")
 	var drNameDupErr = fmt.Errorf("date ranges contains duplicate name 'testName'")
 	var nrNameDupErr = fmt.Errorf("numeric ranges contains duplicate name 'testName'")
 	value := float64(5)
@@ -2767,3 +2767,230 @@ func TestDateRangeStringQuery(t *testing.T) {
 		}
 	}
 }
+
+func TestDateRangeFaceQueriesWithCustomDateTimeParser(t *testing.T) {
+	idxMapping := NewIndexMapping()
+
+	err := idxMapping.AddCustomDateTimeParser("customDT", map[string]interface{}{
+		"type": sanitized.Name,
+		"layouts": []interface{}{
+			"02/01/2006 15:04:05",
+			"2006/01/02 3:04PM",
+		},
+	})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = idxMapping.AddCustomDateTimeParser("queryDT", map[string]interface{}{
+		"type": sanitized.Name,
+		"layouts": []interface{}{
+			"02/01/2006 3:04PM",
+		},
+	})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dtmap := NewDateTimeFieldMapping()
+	dtmap.DateFormat = "customDT"
+	idxMapping.DefaultMapping.AddFieldMappingsAt("date", dtmap)
+
+	tmpIndexPath := createTmpIndexPath(t)
+	defer cleanupTmpIndexPath(t, tmpIndexPath)
+
+	idx, err := New(tmpIndexPath, idxMapping)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		err = idx.Close()
+		if err != nil {
+			t.Fatal(err)
+		}
+	}()
+	documents := map[string]map[string]interface{}{
+		"doc1": {
+			"date": "2001/08/20 6:00PM",
+		},
+		"doc2": {
+			"date": "20/08/2001 18:00:20",
+		},
+		"doc3": {
+			"date": "20/08/2001 18:10:00",
+		},
+		"doc4": {
+			"date": "2001/08/20 6:15PM",
+		},
+		"doc5": {
+			"date": "20/08/2001 18:20:00",
+		},
+	}
+
+	batch := idx.NewBatch()
+	for docID, doc := range documents {
+		err := batch.Index(docID, doc)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	err = idx.Batch(batch)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	query := NewMatchAllQuery()
+
+	type testFacetResult struct {
+		name  string
+		start string
+		end   string
+		count int
+		err   error
+	}
+
+	type testFacetRequest struct {
+		name   string
+		start  string
+		end    string
+		parser string
+		result testFacetResult
+	}
+
+	tests := []testFacetRequest{
+		{
+			// Test without a query time override of the parser (use default parser)
+			name:  "test",
+			start: "2001-08-20 18:00:00",
+			end:   "2001-08-20 18:10:00",
+			result: testFacetResult{
+				name:  "test",
+				start: "2001-08-20 18:00:00",
+				end:   "2001-08-20 18:10:00",
+				count: 2,
+				err:   nil,
+			},
+		},
+		{
+			name:   "test",
+			start:  "20/08/2001 6:00PM",
+			end:    "20/08/2001 6:10PM",
+			parser: "queryDT",
+			result: testFacetResult{
+				name:  "test",
+				start: "20/08/2001 6:00PM",
+				end:   "20/08/2001 6:10PM",
+				count: 2,
+				err:   nil,
+			},
+		},
+		{
+			name:   "test",
+			start:  "20/08/2001 15:00:00",
+			end:    "2001/08/20 6:10PM",
+			parser: "customDT",
+			result: testFacetResult{
+				name:  "test",
+				start: "20/08/2001 15:00:00",
+				end:   "2001/08/20 6:10PM",
+				count: 2,
+				err:   nil,
+			},
+		},
+		{
+			name:   "test",
+			end:    "2001/08/20 6:15PM",
+			parser: "customDT",
+			result: testFacetResult{
+				name:  "test",
+				end:   "2001/08/20 6:15PM",
+				count: 3,
+				err:   nil,
+			},
+		},
+		{
+			name:   "test",
+			start:  "20/08/2001 6:15PM",
+			parser: "queryDT",
+			result: testFacetResult{
+				name:  "test",
+				start: "20/08/2001 6:15PM",
+				count: 2,
+				err:   nil,
+			},
+		},
+		// some error cases
+		{
+			name:   "test",
+			parser: "queryDT",
+			result: testFacetResult{
+				name: "test",
+				err:  fmt.Errorf("date range query must specify either start, end or both for date range name 'test'"),
+			},
+		},
+		{
+			// default parser is used for the query, but the start time is not in the correct format (RFC3339),
+			// so it should throw an error
+			name:  "test",
+			start: "20/08/2001 6:15PM",
+			result: testFacetResult{
+				name: "test",
+				err:  fmt.Errorf("ParseDates err: error parsing start date '20/08/2001 6:15PM' for date range name 'test': unable to parse datetime with any of the layouts, using date time parser named dateTimeOptional"),
+			},
+		},
+	}
+
+	for _, test := range tests {
+		searchRequest := NewSearchRequest(query)
+
+		fr := NewFacetRequest("date", 100)
+		start := &test.start
+		if test.start == "" {
+			start = nil
+		}
+		end := &test.end
+		if test.end == "" {
+			end = nil
+		}
+
+		fr.AddDateTimeRangeStringWithParser(test.name, start, end, test.parser)
+		searchRequest.AddFacet("dateFacet", fr)
+
+		searchResults, err := idx.Search(searchRequest)
+		if err != nil {
+			if test.result.err == nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if err.Error() != test.result.err.Error() {
+				t.Fatalf("Expected error %v, got %v", test.result.err, err)
+			}
+			continue
+		}
+		for _, facetResult := range searchResults.Facets {
+			if len(facetResult.DateRanges) != 1 {
+				t.Fatal("Expected 1 date range facet")
+			}
+			result := facetResult.DateRanges[0]
+			if result.Name != test.result.name {
+				t.Fatalf("Expected name %s, got %s", test.result.name, result.Name)
+			}
+			if result.Start != nil && *result.Start != test.result.start {
+				t.Fatalf("Expected start %s, got %s", test.result.start, *result.Start)
+			}
+			if result.End != nil && *result.End != test.result.end {
+				t.Fatalf("Expected end %s, got %s", test.result.end, *result.End)
+			}
+			if result.Start == nil && test.result.start != "" {
+				t.Fatalf("Expected start %s, got nil", test.result.start)
+			}
+			if result.End == nil && test.result.end != "" {
+				t.Fatalf("Expected end %s, got nil", test.result.end)
+			}
+			if result.Count != test.result.count {
+				t.Fatalf("Expected count %d, got %d", test.result.count, result.Count)
+			}
+		}
+	}
+}
diff --git a/test/tests/facet/searches.json b/test/tests/facet/searches.json
index 6752282a..33ac3977 100644
--- a/test/tests/facet/searches.json
+++ b/test/tests/facet/searches.json
@@ -129,12 +129,12 @@
 						{
 							"name": "new",
 							"count": 9,
-							"start": "2012-01-01T00:00:00Z"
+							"start": "2012-01-01"
 						},
 						{
 							"name": "old",
 							"count": 1,
-							"end": "2012-01-01T00:00:00Z"
+							"end": "2012-01-01"
 						}
 					]
 				}