-
Notifications
You must be signed in to change notification settings - Fork 987
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
graph,graphql,store: nested sorting #3096
Conversation
e3e551a
to
ad81c2b
Compare
980d3a8
to
2c84c57
Compare
ad81c2b
to
c288e18
Compare
2c84c57
to
578da46
Compare
c288e18
to
c02173a
Compare
578da46
to
5757058
Compare
c02173a
to
9afb13b
Compare
5757058
to
adc77ed
Compare
9afb13b
to
46d2872
Compare
adc77ed
to
6565873
Compare
46d2872
to
771a995
Compare
6565873
to
1b71839
Compare
809b6fd
to
f34918a
Compare
f34918a
to
2a77b5d
Compare
1e9f27b
to
5523cf7
Compare
6e04d05
to
6f9312f
Compare
b863adb
to
461a827
Compare
461a827
to
2c30a78
Compare
5e8be64
to
cce0b3b
Compare
230c169
to
1d9d19d
Compare
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 started reviewing this, but then realized that it also deals with sorting by interface fields, and now I am not certain how it relates to PR #4058. Should I continue reviewing this or is this superseded by that PR?
1d9d19d
to
4f1adae
Compare
It deals with sorting by interface fields, #4058 adds sorting an interface by children (as it's a bit more complex). |
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 am still not completely through the PR, but figured these comments would be good to address to make some progress.
@lutter about the failing test, it works locally but I will try to use exact version on PostgreSQL docker image. |
Rows of Sometimes the unit test is 🟢, but sometimes it's 🔴 . Do you think that the order of row insertion affects the end result? These two rows are pretty much identical (
After running the unit tests, I run the query generated by graph-node: select 'Musician' as entity, to_jsonb(c.*) as data from (select c.*
from "sgd21"."musician" c left join "sgd21"."band" as cc on (cc."id" = c."main_band" and cc."block_range" @> 1)
where c.block_range @> 1
order by cc."id", cc.block_range
limit 100) c I always get consistent results. |
I am not entirely sure what is going on here; for me locally, |
4b0a0f5
to
1d343e1
Compare
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.
Nice work! Sorry it took so long to review.
One final thing: we should have an environment variable GRAPH_GRAPHQL_DISABLE_CHILD_SORTING
which defaults to false
that we can set to turn this off if it leads to performance issues. The flag should work just the same as GRAPH_GRAPHQL_DISABLE_BOOL_FILTERS
does. I left comments where I think this needs to be checked to (a) suppress child sorting in the GraphQL schema and (b) return an error in queries.
Other than that, this should be good to go.
1d343e1
to
7910ff1
Compare
graphql: test orderBy enum graphql: do not sort an interface by child-level entity graphql: merge Object and Interface match case graphql: Do not pass field type store: avoid expect Avoid format macro Make ChildKeyDetails more expressive Use constraint_violation macro Sorting by child id Require less data Refactor EntityOrderByChild* Support sorting by child id Add GRAPH_GRAPHQL_DISABLE_CHILD_SORTING (false by default)
7910ff1
to
78adcbb
Compare
@lutter merged with the integration tests failing, it seems like the same on |
#3737
Given this schema:
It adds fields of child entity to
*_orderBy
enum:type Child @entity { name: String }
type Child @entity { id: ID }
type Child @entity { names: [String] }
type Child @entity { secondLevelChild: AnotherEntity }
type Child @entity { secondLevelChildren: [AnotherEntity] }
@derivedFrom
interface Child { name: String }
interface Child { id: ID }
type Child @entity { name: String }
(done in Nested Sorting: interface by non-derived child entity #4058)