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

Functionality to find first N results #77

Merged
merged 8 commits into from
May 3, 2016
Merged

Functionality to find first N results #77

merged 8 commits into from
May 3, 2016

Conversation

manishrjain
Copy link
Contributor

@manishrjain manishrjain commented May 3, 2016 •
edited
Loading

This change is Reviewable

@manishrjain manishrjain added the kind/feature Something completely new we should consider. label May 3, 2016
@manishrjain manishrjain added this to the v0.3 milestone May 3, 2016
@manishrjain
Copy link
Contributor Author

Support for LimitBy

@pawanrawal
Copy link
Contributor

Review status: 0 of 8 files reviewed at latest revision, 5 unresolved discussions.

gql/parser.go, line 159 [r2] (raw file):
Is this space intentional ?

gql/parser.go, line 162 [r2] (raw file):
Is this space intentional ?

posting/list.go, line 743 [r2] (raw file):
Is this space needed ?

posting/list.go, line 744 [r2] (raw file):
Would it be a good idea to combine the if and else if blocks here?

query/query.go, line 114 [r2] (raw file):
Are we using this anywhere? I suppose this is for later when we provide the user​ option to set an offset?

Comments from Reviewable

@manishrjain
Copy link
Contributor Author

Review status: 0 of 8 files reviewed at latest revision, 5 unresolved discussions.

gql/parser.go, line 159 [r2] (raw file):
I typically do it just before an else statement, to make the conditions clear.

gql/parser.go, line 162 [r2] (raw file):
Yes, same reason as above.

posting/list.go, line 743 [r2] (raw file):
Removed if else.

posting/list.go, line 744 [r2] (raw file):
Done.

query/query.go, line 114 [r2] (raw file):
Yeah -- It's not required right ​now, but the code is set up to use it readily.

Comments from Reviewable

@pawanrawal
Copy link
Contributor

:lgtm:

Review status: 0 of 8 files reviewed at latest revision, 1 unresolved discussion.

query/query.go, line 114 [r2] (raw file):
Cool

Comments from Reviewable

@pawanrawal
Copy link
Contributor

:lgtm:

Review status: 0 of 8 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@pawanrawal
Copy link
Contributor

Reviewed 6 of 8 files at r1, 1 of 1 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@manishrjain manishrjain merged commit d726dea into master May 3, 2016
@manishrjain manishrjain added the kind/feature Something completely new we should consider. label Mar 22, 2018
arijitAD pushed a commit that referenced this pull request Oct 15, 2020
@dutterbutter @noot
dependency management: dep (#77)
8ffae65 
Added dep to be used for dependency management until go mods are more widely supported
shivaji-dgraph pushed a commit that referenced this pull request Mar 12, 2024
@sunilmujumdar
Bug(GraphQL/Schema): If a type had an embedding as well as a list, hm…
c99233b 
…_distance field was getting clobbered (#77)

Description: 

```
querySimilar<Type> queries defined a new derived type "<Type>WithDistance" with a new field hm_distance.

However, if <Type> had any lists, hm_distance was getting clobbered by an "<ListFieldName>Aggregate" being added to <Type>

The fix essentially does away with the derived type <Type>WithDistance as the resultType for querySimilar<Type> queries. Instead, we add <embeddingFieldName>Distance field for each embedding in the <Type> definition itself. This would make it easy to add support for filters on embeddings.
```

Fixes: HYP-447
harshil-goel pushed a commit that referenced this pull request Mar 12, 2024
@sunilmujumdar
Bug(GraphQL/Schema): If a type had an embedding as well as a list, hm…
bc8da9b 
…_distance field was getting clobbered (#77)

Description: 

```
querySimilar<Type> queries defined a new derived type "<Type>WithDistance" with a new field hm_distance.

However, if <Type> had any lists, hm_distance was getting clobbered by an "<ListFieldName>Aggregate" being added to <Type>

The fix essentially does away with the derived type <Type>WithDistance as the resultType for querySimilar<Type> queries. Instead, we add <embeddingFieldName>Distance field for each embedding in the <Type> definition itself. This would make it easy to add support for filters on embeddings.
```

Fixes: HYP-447
harshil-goel pushed a commit that referenced this pull request Apr 17, 2024
@sunilmujumdar @harshil-goel
Bug(GraphQL/Schema): If a type had an embedding as well as a list, hm…
7b6080b 
…_distance field was getting clobbered (#77)

Description: 

```
querySimilar<Type> queries defined a new derived type "<Type>WithDistance" with a new field hm_distance.

However, if <Type> had any lists, hm_distance was getting clobbered by an "<ListFieldName>Aggregate" being added to <Type>

The fix essentially does away with the derived type <Type>WithDistance as the resultType for querySimilar<Type> queries. Instead, we add <embeddingFieldName>Distance field for each embedding in the <Type> definition itself. This would make it easy to add support for filters on embeddings.
```

Fixes: HYP-447
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pawanrawal pawanrawal

Labels
kind/feature Something completely new we should consider.
Milestone
v0.3
Development

Successfully merging this pull request may close these issues.
2 participants
@manishrjain @pawanrawal