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.

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 support for full-text search #192

Merged
merged 1 commit into from
Dec 21, 2015
Merged

Add support for full-text search #192

merged 1 commit into from
Dec 21, 2015

Conversation

fatlotus
Copy link
Member

Also fairly rudimentary. This brings us (almost) up to parity with the old site.

memcache.set_multi(writeback, time=3600, key_prefix=FTS)

# Elide potentially stale entries from the cache.
for candidate in ndb.get_multi([key for key in keys if key in matches]):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this bit?

It seems to me that keys will be the keys for the last word in words from the for loop in line 28.
Do you want to aggregate al keys from all the words first?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're doing something sorta tricky. (First, keys will never be null since words is non-empty.) Each keys is in order, so we iterate through one of them — but remove any that aren't in all matches (if key in matches). Hence we get a list of keys, in order, that contain each word.

I'll see if I can add a comment to that effect since it's certainly not explained well as-is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just take matches then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Matches is a set so it might not be in order.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay.

georgeteo added a commit that referenced this pull request Dec 21, 2015
@georgeteo georgeteo merged commit 1a47289 into master Dec 21, 2015
@georgeteo georgeteo deleted the add-full-text-search branch December 21, 2015 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants