Bug fixing: query isCron error (uber#6222)

* Bug fix: isCron return error

* add more tests

* add more tests

* run fmt

common/definition/indexedKeys.go

@@ -110,3 +110,8 @@ func IsSystemIndexedKey(key string) bool {
 	_, ok := systemIndexedKeys[key]
 	return ok
 }
+
+// IsSystemBoolKey return true is key is system added bool key
+func IsSystemBoolKey(key string) bool {
+	return systemIndexedKeys[key] == types.IndexedValueTypeBool
+}

common/definition/indexedKeys_test.go

@@ -0,0 +1,44 @@
+// The MIT License (MIT)
+
+// Copyright (c) 2017-2020 Uber Technologies Inc.
+
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+package definition
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsSystemBoolKey(t *testing.T) {
+	tests := []struct {
+		key      string
+		expected bool
+	}{
+		{"IsCron", true},
+		{"StartTime", false},
+	}
+
+	for _, test := range tests {
+		actualResult := IsSystemBoolKey(test.key)
+		assert.Equal(t, test.expected, actualResult)
+	}
+}

common/pinot/pinotQueryValidator.go

@@ -232,6 +232,24 @@ func (qv *VisibilityQueryValidator) IsValidSearchAttributes(key string) bool {
 	return isValidKey
 }
 
+func (qv *VisibilityQueryValidator) processSystemBoolKey(colNameStr string, comparisonExpr sqlparser.ComparisonExpr) (string, error) {
+	// case1: isCron = false
+	colVal, ok := comparisonExpr.Right.(sqlparser.BoolVal)
+	if !ok {
+		// case2: isCron = "false" or isCron = 'false'
+		sqlVal, ok := comparisonExpr.Right.(*sqlparser.SQLVal)
+		if !ok {
+			return "", fmt.Errorf("failed to process a bool key to SQLVal: %v", comparisonExpr.Right)
+		}
+		colValStr := string(sqlVal.Val)
+		if strings.ToLower(colValStr) != "false" && strings.ToLower(colValStr) != "true" {
+			return "", fmt.Errorf("invalid bool value in pinot_query_validator: %s", colValStr)
+		}
+		return fmt.Sprintf("%s = %s", colNameStr, colValStr), nil
+	}
+	return fmt.Sprintf("%s = %v", colNameStr, colVal), nil
+}
+
 func (qv *VisibilityQueryValidator) processSystemKey(expr sqlparser.Expr) (string, error) {
 	comparisonExpr := expr.(*sqlparser.ComparisonExpr)
 	buf := sqlparser.NewTrackedBuffer(nil)
@@ -242,6 +260,11 @@ func (qv *VisibilityQueryValidator) processSystemKey(expr sqlparser.Expr) (strin
 	}
 	colNameStr := colName.Name.String()
 
+	// handle system bool key
+	if definition.IsSystemBoolKey(colNameStr) {
+		return qv.processSystemBoolKey(colNameStr, *comparisonExpr)
+	}
+
 	if comparisonExpr.Operator == sqlparser.LikeStr {
 		colVal, ok := comparisonExpr.Right.(*sqlparser.SQLVal)
 		if !ok {

common/pinot/pinotQueryValidator_test.go

@@ -279,6 +279,28 @@ func TestValidateQuery(t *testing.T) {
 			validated: "",
 			err:       "invalid IN expression, value",
 		},
+		"case21-1: test bool value- system key- no quotes": {
+			query:     "IsCron = true",
+			validated: "IsCron = true",
+		},
+		"case21-2: test bool value- system key- single quotes": {
+			query:     "IsCron = 'true'",
+			validated: "IsCron = true",
+		},
+		"case21-3: test bool value- system key- double quotes": {
+			query:     "IsCron = \"true\"",
+			validated: "IsCron = true",
+		},
+		"case21-4: test bool value- system key- invalid value": {
+			query:     "IsCron = 1",
+			validated: "",
+			err:       "invalid bool value in pinot_query_validator: 1",
+		},
+		"case21-5: test bool value- when it is not SQLBool and SQLVAl": {
+			query:     "IsCron = abc",
+			validated: "",
+			err:       "failed to process a bool key to SQLVal: &{<nil> abc { }}",
+		},
 	}
 
 	for name, test := range tests {