IG-22140, IG-22141: Format key values to match python (#651)

* IG-22140, IG-22141: Format key values to match python * Use `valueToTypedExpressionString` in `formatKeyName` * Fix * Try fix for linter * Move requirement * Keep existing string formatting of timestamps * Add regression tests * lint again --------- Co-authored-by: Gal Topper <[email protected]>
v3io · Sep 21, 2023 · 21f0e9f · 21f0e9f
1 parent 404c6b4
commit 21f0e9f
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 21 deletions.
diff --git a/backends/kv/writer.go b/backends/kv/writer.go
@@ -363,13 +363,12 @@ func (a *Appender) Add(frame frames.Frame) error {
 }
 
 func (a *Appender) formatKeyName(key interface{}, sortingVal interface{}) string {
-	var format string
+	var formattedKey string
+	formattedKey = valueToKeyString(key)
 	if sortingVal != nil {
-		format = fmt.Sprintf("%v.%v", key, sortingVal)
-	} else {
-		format = fmt.Sprintf("%v", key)
+		formattedKey = fmt.Sprintf("%s.%s", formattedKey, valueToKeyString(sortingVal))
 	}
-	return format
+	return formattedKey
 }
 
 // update updates rows from a frame
@@ -685,3 +684,28 @@ func valueToTypedExpressionString(value interface{}) string {
 		return fmt.Sprintf("%v", value)
 	}
 }
+
+func valueToKeyString(value interface{}) string {
+	switch typedVal := value.(type) {
+	case string:
+		return typedVal
+	case bool:
+		// Python/storey compatibility by capitalizing
+		// IG-22141
+		if typedVal {
+			return "True"
+		}
+		return "False"
+	case float64, float32:
+		// Python/storey compatibility by deleting trailing zeros (except one trailing zero after period)
+		// IG-22140
+		str := fmt.Sprintf("%f", typedVal)
+		str = strings.TrimRight(str, "0")
+		if strings.HasSuffix(str, ".") {
+			str += "0"
+		}
+		return str
+	default:
+		return fmt.Sprintf("%v", value)
+	}
+}
diff --git a/clients/py/dev-requirements.txt b/clients/py/dev-requirements.txt
@@ -1,4 +1,6 @@
 build
+# https://stackoverflow.com/questions/73929564/entrypoints-object-has-no-attribute-get-digital-ocean
+importlib-metadata<5
 flake8
 ipython
 numpydoc

diff --git a/test/kv_integration_test.go b/test/kv_integration_test.go
@@ -180,6 +180,24 @@ func (kvSuite *KvTestSuite) TestWriteToExistingFolderWithoutSchema() {
 
 }
 
+func (kvSuite *KvTestSuite) generateColumn(columnName string, columnType string, size int) frames.Column {
+	switch columnType {
+	case "string":
+		return StringCol(kvSuite.T(), columnName, size)
+	case "float":
+		return FloatCol(kvSuite.T(), columnName, size)
+	case "bool":
+		return BoolCol(kvSuite.T(), columnName, size)
+	case "time":
+		return TimeCol(kvSuite.T(), columnName, size)
+	case "int":
+		return IntCol(kvSuite.T(), columnName, size)
+	default:
+		kvSuite.T().Fatalf("type %v not supported", columnType)
+		return nil // no-op
+	}
+}
+
 func (kvSuite *KvTestSuite) generateSequentialSampleFrameWithTypes(size int, indexName string, columnNames map[string]string) frames.Frame {
 	var icol frames.Column
 
@@ -194,21 +212,27 @@ func (kvSuite *KvTestSuite) generateSequentialSampleFrameWithTypes(size int, ind
 	columns := make([]frames.Column, len(columnNames))
 	i := 0
 	for columnName, columnType := range columnNames {
-		switch columnType {
-		case "string":
-			columns[i] = StringCol(kvSuite.T(), columnName, size)
-		case "float":
-			columns[i] = FloatCol(kvSuite.T(), columnName, size)
-		case "bool":
-			columns[i] = BoolCol(kvSuite.T(), columnName, size)
-		case "time":
-			columns[i] = TimeCol(kvSuite.T(), columnName, size)
-		case "int":
-			columns[i] = IntCol(kvSuite.T(), columnName, size)
-		default:
-			kvSuite.T().Fatalf("type %v not supported", columnType)
-		}
+		columns[i] = kvSuite.generateColumn(columnName, columnType, size)
+		i++
+	}
+
+	frame, err := frames.NewFrame(columns, []frames.Column{icol}, nil)
+	kvSuite.Require().NoError(err)
+
+	return frame
+}
+
+func (kvSuite *KvTestSuite) generateSequentialSampleFrameWithTypesV2(size int, indexColumnName string, columnNames map[string]string) frames.Frame {
+	indexColumnType, ok := columnNames[indexColumnName]
+	kvSuite.Require().True(ok)
+	delete(columnNames, indexColumnName)
 
+	icol := kvSuite.generateColumn(indexColumnName, indexColumnType, size)
+
+	columns := make([]frames.Column, len(columnNames))
+	i := 0
+	for columnName, columnType := range columnNames {
+		columns[i] = kvSuite.generateColumn(columnName, columnType, size)
 		i++
 	}
 
@@ -218,6 +242,90 @@ func (kvSuite *KvTestSuite) generateSequentialSampleFrameWithTypes(size int, ind
 	return frame
 }
 
+// IG-22141
+func (kvSuite *KvTestSuite) TestFloatIndexColumn() {
+	table := fmt.Sprintf("frames_ci/TestFloatIndexColumn%d", time.Now().UnixNano())
+
+	columnNames := map[string]string{"idx": "float", "n": "int"}
+	frame := kvSuite.generateSequentialSampleFrameWithTypesV2(3, "idx", columnNames)
+	wreq := &frames.WriteRequest{
+		Backend: kvSuite.backendName,
+		Table:   table,
+	}
+
+	appender, err := kvSuite.client.Write(wreq)
+	kvSuite.Require().NoError(err)
+
+	err = appender.Add(frame)
+	kvSuite.Require().NoError(err)
+
+	err = appender.WaitForComplete(10 * time.Second)
+	kvSuite.Require().NoError(err)
+
+	input := v3io.GetItemsInput{AttributeNames: []string{"__name", "n"}}
+
+	iter, err := v3ioutils.NewAsyncItemsCursor(
+		kvSuite.v3ioContainer, &input, 1,
+		nil, kvSuite.internalLogger,
+		0, []string{table + "/"},
+		"", "")
+
+	for iter.Next() {
+		currentRow := iter.GetItem()
+
+		key, _ := currentRow.GetFieldString("__name")
+		switch key {
+		case ".#schema":
+			continue
+		default:
+			name := currentRow.GetField("__name").(string)
+			kvSuite.Require().True(strings.HasSuffix(name, ".0"))
+		}
+	}
+}
+
+// IG-22140
+func (kvSuite *KvTestSuite) TestBoolIndexColumn() {
+	table := fmt.Sprintf("frames_ci/TestFloatIndexColumn%d", time.Now().UnixNano())
+
+	columnNames := map[string]string{"idx": "bool", "n": "int"}
+	frame := kvSuite.generateSequentialSampleFrameWithTypesV2(1, "idx", columnNames)
+	wreq := &frames.WriteRequest{
+		Backend: kvSuite.backendName,
+		Table:   table,
+	}
+
+	appender, err := kvSuite.client.Write(wreq)
+	kvSuite.Require().NoError(err)
+
+	err = appender.Add(frame)
+	kvSuite.Require().NoError(err)
+
+	err = appender.WaitForComplete(10 * time.Second)
+	kvSuite.Require().NoError(err)
+
+	input := v3io.GetItemsInput{AttributeNames: []string{"__name", "n"}}
+
+	iter, err := v3ioutils.NewAsyncItemsCursor(
+		kvSuite.v3ioContainer, &input, 1,
+		nil, kvSuite.internalLogger,
+		0, []string{table + "/"},
+		"", "")
+
+	for iter.Next() {
+		currentRow := iter.GetItem()
+
+		key, _ := currentRow.GetFieldString("__name")
+		switch key {
+		case ".#schema":
+			continue
+		default:
+			name := currentRow.GetField("__name").(string)
+			kvSuite.Require().Equal(name, "True")
+		}
+	}
+}
+
 func (kvSuite *KvTestSuite) TestAll() {
 	table := fmt.Sprintf("frames_ci/kv_test_all%d", time.Now().UnixNano())
 

diff --git a/test/test_utils.go b/test/test_utils.go
@@ -36,10 +36,9 @@ import (
 type SuiteCreateFunc = func(frames.Client, v3io.Container, logger.Logger) suite.TestingSuite
 
 func FloatCol(t testing.TB, name string, size int) frames.Column {
-	random := rand.New(rand.NewSource(time.Now().UnixNano()))
 	floats := make([]float64, size)
 	for i := range floats {
-		floats[i] = random.Float64()
+		floats[i] = float64(i)
 	}
 
 	col, err := frames.NewSliceColumn(name, floats)