Repository: oom-ai/oomstore
Pull request #869 (2021-12-30 09:12:01)

feature: online sqlite implement method push
this PR does:
1. refactor sqlutil.BuildPushCondition: wrap return values to a struct
2. sqlite implements the push method

relate #801 

YikSanChan: @lianxmfor PR title can be more descriptive, include "sqlite"
codecov[bot]: # [Codecov](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai) Report
> Merging [#869](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai) (b3e5181) into [main](https://codecov.io/gh/oom-ai/oomstore/commit/d330ca24b4e52bd0ff7e41920a00795e49d60e5a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai) (d330ca2) will **increase** coverage by `0.16%`.
> The diff coverage is `87.50%`.

[![Impacted file tree graph](https://codecov.io/gh/oom-ai/oomstore/pull/869/graphs/tree.svg?width=650&height=150&src=pr&token=C59L7LTRM4&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai)](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai)

```diff
@@            Coverage Diff             @@
##             main     #869      +/-   ##
==========================================
+ Coverage   50.75%   50.92%   +0.16%     
==========================================
  Files          82       82              
  Lines        3294     3307      +13     
==========================================
+ Hits         1672     1684      +12     
  Misses       1426     1426              
- Partials      196      197       +1     
```


| [Impacted Files](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai) | Coverage Δ | |
|---|---|---|
| [internal/database/online/sqlite/store.go](https://codecov.io/gh/oom-ai/oomstore/pull/869/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai#diff-aW50ZXJuYWwvZGF0YWJhc2Uvb25saW5lL3NxbGl0ZS9zdG9yZS5nbw==) | `80.00% <78.57%> (+9.41%)` | :arrow_up: |
| [internal/database/online/mysql/store.go](https://codecov.io/gh/oom-ai/oomstore/pull/869/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai#diff-aW50ZXJuYWwvZGF0YWJhc2Uvb25saW5lL215c3FsL3N0b3JlLmdv) | `90.32% <100.00%> (ø)` | |
| [internal/database/online/postgres/store.go](https://codecov.io/gh/oom-ai/oomstore/pull/869/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai#diff-aW50ZXJuYWwvZGF0YWJhc2Uvb25saW5lL3Bvc3RncmVzL3N0b3JlLmdv) | `90.62% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai). Last update [d330ca2...b3e5181](https://codecov.io/gh/oom-ai/oomstore/pull/869?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oom-ai).


Patch:
diff --git a/internal/database/online/mysql/store.go b/internal/database/online/mysql/store.go
index f281706b..2e230e14 100644
--- a/internal/database/online/mysql/store.go
+++ b/internal/database/online/mysql/store.go
@@ -52,19 +52,19 @@ func (db *DB) Purge(ctx context.Context, revisionID int) error {
 func (db *DB) Push(ctx context.Context, opt online.PushOpt) error {
 	tableName := sqlutil.OnlineStreamTableName(opt.GroupID)
 
-	inserts, insertPlaceholders, updatePlaceholders, values, err := sqlutil.BuildPushCondition(opt, Backend)
+	cond, err := sqlutil.BuildPushCondition(opt, Backend)
 	if err != nil {
 		return err
 	}
 
 	query := fmt.Sprintf(`INSERT INTO %s (%s) VALUES(%s) ON DUPLICATE KEY UPDATE %s`,
 		tableName,
-		inserts,
-		insertPlaceholders,
-		updatePlaceholders,
+		cond.Inserts,
+		cond.InsertPlaceholders,
+		cond.UpdatePlaceholders,
 	)
 
-	_, err = db.ExecContext(ctx, db.Rebind(query), values...)
+	_, err = db.ExecContext(ctx, db.Rebind(query), append(cond.InsertValues, cond.UpdateValues...)...)
 	return err
 }
 
diff --git a/internal/database/online/postgres/store.go b/internal/database/online/postgres/store.go
index d569eb88..b35f4596 100644
--- a/internal/database/online/postgres/store.go
+++ b/internal/database/online/postgres/store.go
@@ -51,20 +51,20 @@ func (db *DB) Purge(ctx context.Context, revisionID int) error {
 func (db *DB) Push(ctx context.Context, opt online.PushOpt) error {
 	tableName := sqlutil.OnlineStreamTableName(opt.GroupID)
 
-	inserts, insertPlaceholders, updatePlaceholders, values, err := sqlutil.BuildPushCondition(opt, Backend)
+	cond, err := sqlutil.BuildPushCondition(opt, Backend)
 	if err != nil {
 		return err
 	}
 
 	query := fmt.Sprintf(`INSERT INTO %s(%s) VALUES(%s) ON CONFLICT ("%s") DO UPDATE SET %s`,
 		tableName,
-		inserts,
-		insertPlaceholders,
+		cond.Inserts,
+		cond.InsertPlaceholders,
 		opt.Entity.Name,
-		updatePlaceholders,
+		cond.UpdatePlaceholders,
 	)
 
-	_, err = db.ExecContext(ctx, db.Rebind(query), values...)
+	_, err = db.ExecContext(ctx, db.Rebind(query), append(cond.InsertValues, cond.UpdateValues...)...)
 	return err
 }
 
diff --git a/internal/database/online/sqlite/store.go b/internal/database/online/sqlite/store.go
index 9ea088bb..435e7364 100644
--- a/internal/database/online/sqlite/store.go
+++ b/internal/database/online/sqlite/store.go
@@ -2,6 +2,7 @@ package sqlite
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/jmoiron/sqlx"
 
@@ -47,7 +48,21 @@ func (db *DB) Purge(ctx context.Context, revisionID int) error {
 }
 
 func (db *DB) Push(ctx context.Context, opt online.PushOpt) error {
-	panic("Implement me!")
+	tableName := sqlutil.OnlineStreamTableName(opt.GroupID)
+
+	cond, err := sqlutil.BuildPushCondition(opt, Backend)
+	if err != nil {
+		return err
+	}
+
+	query := fmt.Sprintf("REPLACE INTO %s (%s) VALUES(%s)",
+		tableName,
+		cond.Inserts,
+		cond.InsertPlaceholders,
+	)
+
+	_, err = db.ExecContext(ctx, db.Rebind(query), cond.InsertValues...)
+	return err
 }
 
 func (db *DB) PrepareStreamTable(ctx context.Context, opt online.PrepareStreamTableOpt) error {
diff --git a/internal/database/online/sqlutil/get.go b/internal/database/online/sqlutil/get.go
index 4ed1862f..b8ac99a3 100644
--- a/internal/database/online/sqlutil/get.go
+++ b/internal/database/online/sqlutil/get.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 
 	"github.com/jmoiron/sqlx"
-	"github.com/spf13/cast"
 
 	"github.com/oom-ai/oomstore/internal/database/dbutil"
 	"github.com/oom-ai/oomstore/internal/database/online"
@@ -94,7 +93,6 @@ func deserializeByTag(i interface{}, valueType types.ValueType, backend types.Ba
 		return nil, nil
 	}
 
-	// TODO: merge with DeserializeByValueType in internal/database/offline/sqlutil/serialize.go
 	switch valueType {
 	case types.String:
 		if backend == types.BackendMySQL {
@@ -103,10 +101,9 @@ func deserializeByTag(i interface{}, valueType types.ValueType, backend types.Ba
 		return i, nil
 	case types.Bool:
 		if backend == types.BackendMySQL || backend == types.BackendSQLite {
-			s := cast.ToString(i)
-			if s == "1" || s == "true" {
+			if i == int64(1) {
 				return true, nil
-			} else if s == "0" || s == "false" {
+			} else if i == int64(0) {
 				return false, nil
 			} else {
 				return nil, fmt.Errorf("invalid bool value: %s", i)
diff --git a/internal/database/online/sqlutil/push.go b/internal/database/online/sqlutil/push.go
index 9cc063ba..45d3ad85 100644
--- a/internal/database/online/sqlutil/push.go
+++ b/internal/database/online/sqlutil/push.go
@@ -9,24 +9,32 @@ import (
 	"github.com/oom-ai/oomstore/pkg/oomstore/types"
 )
 
-func BuildPushCondition(opt online.PushOpt, backend types.BackendType) (string, string, string, []interface{}, error) {
+type PushCondition struct {
+	Inserts            string
+	InsertPlaceholders string
+	InsertValues       []interface{}
+	UpdateValues       []interface{}
+	UpdatePlaceholders string
+}
+
+func BuildPushCondition(opt online.PushOpt, backend types.BackendType) (*PushCondition, error) {
 	qt, err := dbutil.QuoteFn(backend)
 	if err != nil {
-		return "", "", "", nil, err
+		return nil, err
 	}
 
-	insertColumns := append([]string{opt.Entity.Name}, opt.FeatureNames...)
-	insertValues := append([]interface{}{opt.EntityKey}, opt.FeatureValues...)
+	cond := PushCondition{}
+
+	cond.Inserts = qt(append([]string{opt.Entity.Name}, opt.FeatureNames...)...)
+	cond.InsertValues = append([]interface{}{opt.EntityKey}, opt.FeatureValues...)
+	cond.InsertPlaceholders = dbutil.Fill(len(cond.InsertValues), "?", ",")
 
 	updatePlaceholders := make([]string, 0, len(opt.FeatureNames))
 	for _, name := range opt.FeatureNames {
 		updatePlaceholders = append(updatePlaceholders, fmt.Sprintf("%s=?", qt(name)))
 	}
+	cond.UpdatePlaceholders = strings.Join(updatePlaceholders, ",")
+	cond.UpdateValues = opt.FeatureValues
 
-	return qt(insertColumns...),
-		dbutil.Fill(len(insertColumns), "?", ","),
-		strings.Join(updatePlaceholders, ","),
-		append(insertValues, opt.FeatureValues...),
-		nil
-
+	return &cond, nil
 }


Test patch:
diff --git a/internal/database/online/sqlite/store_test.go b/internal/database/online/sqlite/store_test.go
index 82a7a427..98613403 100644
--- a/internal/database/online/sqlite/store_test.go
+++ b/internal/database/online/sqlite/store_test.go
@@ -56,6 +56,10 @@ func TestPrepareStreamTable(t *testing.T) {
 	test_impl.TestPrepareStreamTable(t, prepareStore, destroyStore)
 }
 
+func TestPush(t *testing.T) {
+	test_impl.TestPush(t, prepareStore, destroyStore)
+}
+
 func TestPing(t *testing.T) {
 	test_impl.TestPing(t, prepareStore, destroyStore)
 }