New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add cmd set #16
Add cmd set #16
Conversation
* SQL queries and method implemented. Signed-off-by: Gökhan Özeloğlu <gozeloglu@gmail.com>
* Update struct and interface definitions. Signed-off-by: Gökhan Özeloğlu <gozeloglu@gmail.com>
Signed-off-by: Gökhan Özeloğlu <gozeloglu@gmail.com>
@@ -113,12 +113,18 @@ type RHash interface { | |||
Values(key string) ([]core.Value, error) | |||
} | |||
|
|||
// RSet is a set repository. | |||
type RSet interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better not to rush things with the command
package. Let's deal with rset
first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't I implement the internal/command
part?
internal/rset/db.go
Outdated
// A set is a field-value map associated with a key. | ||
// Use the set repository to work with individual hashmaps | ||
// and their fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not true for sets. It's OK to copy-paste, but please remember to tailor the code/comments to the feature you're working on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know. This PR is not ready yet. I am going to update it. 👍🏻
tx := NewTx(d.SQL) | ||
return tx.Add(key, elems...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add
modifies the database, so it should be called inside a transaction (see rzset.DB.Add
for reference).
internal/rset/tx.go
Outdated
"time" | ||
) | ||
|
||
var sqlAdd = []string{` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use consts (see rzset
for reference).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I saw a slice of queries in another command. I referred it from there, but sqlAdd1
and sqlAdd2
make sense.
|
||
} | ||
|
||
func (t *Tx) add(key string, elems ...any) (int, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the need for this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that we may need to reuse the same method for other commands, but I might be overengineering. I can remove it.
internal/rset/tx.go
Outdated
for _, elem := range elems { | ||
args = append(args, []any{ | ||
sql.Named("key", key), | ||
sql.Named("type", core.TypeSet), | ||
sql.Named("version", core.InitialVersion), | ||
sql.Named("elem", elem), | ||
sql.Named("etime", now), | ||
sql.Named("mtime", now.UnixMilli()), | ||
}) | ||
} | ||
|
||
for _, arg := range args { | ||
_, err := t.tx.Exec(sqlAdd[0], arg) | ||
if err != nil { | ||
return 0, sqlx.TypedError(err) | ||
} | ||
_, err = t.tx.Exec(sqlAdd[1], arg) | ||
if err != nil { | ||
return 0, err | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not really work.
Signed-off-by: Gökhan Özeloğlu <gozeloglu@gmail.com>
Signed-off-by: Gökhan Özeloğlu <gozeloglu@gmail.com>
Signed-off-by: Gökhan Özeloğlu <gozeloglu@gmail.com>
No description provided.