Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add new command EXPIRE #167

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add new command EXPIRE #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89fc3a2
add new command EXPIRE
Issif Jun 28, 2022
c5b54b0
Merge branch 'master' into feature/expire
Avital-Fine Jul 19, 2022
a62b68a
Merge branch 'master' into feature/expire
Issif Apr 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions redisearch/document.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type Document struct {
Id string
Score float32
Payload []byte
TTL int
Properties map[string]interface{}
}

Expand Down Expand Up @@ -70,6 +71,12 @@ func (d Document) Set(name string, value interface{}) Document {
return d
}

// SetTTL sets the ttl in the document
func (d Document) SetTTL(ttl int) Document {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can definitely see the value... But perhaps we should at least set a default TTL of -1 during the New.

Copy link
Author

@Issif Issif Jul 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the set, it's by default to -1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct - I just prefer being explicit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in holidays, I'll update this PR when I'm back

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought and I think we should let the code like this, in this current setup, if the TTL is not set, we follow the Redis default behavior and the keys have no expiration, if we set the TTL, a second command is run to add an expiration to the key. It means, if we don't need to set the TTL we run only 1 command and not 2. For huge workload, it can be useful to avoid to run 2 commands when it's not necessary. wdyt?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up

d.TTL = ttl
return d
}

// All punctuation marks and whitespaces (besides underscores) separate the document and queries into tokens.
// e.g. any character of `,.<>{}[]"':;!@#$%^&*()-+=~` will break the text into terms.
// So the text `foo-bar.baz...bag` will be tokenized into `[foo, bar, baz, bag]`
Expand Down
12 changes: 12 additions & 0 deletions redisearch/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,18 @@ func (i *Client) IndexOptions(opts IndexingOptions, docs ...Document) error {

return merr
}
if doc.TTL > 0 {
argsttl := make(redis.Args, 0, 2)
argsttl = append(argsttl, doc.Id, doc.TTL)
if err := conn.Send("EXPIRE", argsttl...); err != nil {
if merr == nil {
merr = NewMultiError(len(docs))
}
merr[ii] = err

return merr
}
}
n++
}

Expand Down