-
Notifications
You must be signed in to change notification settings - Fork 3
fix order of results for query requests with more than 10 variable sets #37
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
Conversation
let pipelines_with_response_shapes: BTreeMap<String, (Pipeline, ResponseShape)> = foreach | ||
let pipelines_with_response_shapes: Vec<(String, (Pipeline, ResponseShape))> = foreach |
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 PR contains a lot of fixture and test changes. This line is the only change required for the actual fix.
@@ -160,8 +161,6 @@ impl Connector for MongoConnector { | |||
let response = handle_query_request(state, v2_request) | |||
.await | |||
.map_err(mongo_agent_error_to_query_error)?; | |||
let r = v2_to_v3_query_response(response); | |||
tracing::warn!(v3_response = %serde_json::to_string(&r).unwrap()); |
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 fixes some debugging output I accidentally committed in a previous PR. I'd like to set up the sdk to making the logging level for this crate configurable so that I don't have to change debug!
s to warn!
s to see these traces.
Describe your changes
In query requests with more than 10 variable sets results were sent back in the wrong order because facet fields were put into a
BTreeMap
which sorted them lexicographically. This PR switches to aVec
of tuples to preserve expected order.While working on this I updated the dev services to run two connectors with a remote relationship configured. If you run
arion up -d
you can access GraphiQL at http://localhost:7100/ and test the fix with a query like this one:The remote relationship matches
albumId
in theAlbum
collection of the chinook database with theruntime
field in themovies
collection of the sample_mflix database. It's not a sensical relationship but it gives us something to test with since both of those fields are ints with plenty of values that line up.Issue ticket number and link
MDB-111