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 batch for station routes #42

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

Add batch for station routes #42

wants to merge 3 commits into from

Conversation

antoinecellier
Copy link
Owner

No description provided.

Copy link
Collaborator

@hgwood hgwood left a comment

Choose a reason for hiding this comment

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

Ça me parait intéressant ! Il y a des subtilités à voir pour la gestion de la pagination. Côté GraphQL on donne une pagination pour tous les arrêts, mais ici la pagination fait partie de la clé de chargement donc elle peut potentiellement être différente pour chaque arrêt. Je pense qu'il vaut mieux bien le gérer pour que le code soit clair.

const { from, length } = stopIds[0]
stopIds = map(stopIds, 'stop_id')

return new Promise((resolve, reject) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tu peux retourner directement db().query(...).then(cursor => cursor.all())

Copy link
Owner Author

@antoinecellier antoinecellier Dec 19, 2016

Choose a reason for hiding this comment

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

J'avais tenté cette requête mais sans succès, il ne connait pas stopResolve.from et stopResolve.length

`

   for stopResolve in ${stops} //[{ id, from, length}, { id, from, length}, { id, from, length}]
    let stops = (
      for stop in stops
      filter stop.parent_station == stopResolve.id
      return stop.stop_id
    )

    let stop_times_of_stop = (
      for stop_time in stop_times
      filter stop_time.stop_id in stops
      return stop_time.trip_id
    )

    let routes_of_stops = (
      for trip in trips
      filter trip.trip_id in stop_times_of_stop
      return trip.route_id
    )

    let routes = (
      for route in routes
      filter route.route_id in routes_of_stops
      limit stopResolve.from, stopResolve.length
      return route
    )

  return routes

`

Copy link
Collaborator

Choose a reason for hiding this comment

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

Tu peux regarder dans les logs de la base pour voir quelle requête arrive vraiment à la base. Je pense qu'elle ne doit pas bien contenir l'objet stops.

Copy link
Owner Author

Choose a reason for hiding this comment

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

J'ai loggé cette requête côté arangodb

`
for stopId in @value0
let from = stopId.from

    let stops = (
      for stop in stops
      filter stop.parent_station == stopId
      return stop.stop_id
    )

    let stop_times_of_stop = (
      for stop_time in stop_times
      filter stop_time.stop_id in stops
      return stop_time.trip_id
    )
    let routes_of_stops = (
      for trip in trips
      filter trip.trip_id in stop_times_of_stop
      return trip.route_id
    )
    let routes = (
      for route in routes
      filter route.route_id in routes_of_stops
      limit @value1, @value2
      return route
    )
  return routes

`

Copy link
Collaborator

Choose a reason for hiding this comment

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

Les paramètres n'apparaissent pas ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

2017-01-17T20:20:53Z [7038] DEBUG {queries} bindParameters: {"value0":["StopArea:OTAG","StopArea:MAI8","StopArea:HMVE","StopArea:ABDU","StopArea:ADAP"],"value1":0,"value2":3}

Copy link
Collaborator

Choose a reason for hiding this comment

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

On voit que stops/@value0 n'est pas un tableau d'objet comme tu le voudrais.

Copy link
Owner Author

Choose a reason for hiding this comment

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

2017-01-17T20:41:12Z [8249] DEBUG {queries} bindParameters: {"value0":[{"from":0,"length":3,"stop_id":"StopArea:OTAG"},{"from":0,"length":3,"stop_id":"StopArea:MAI8"},{"from":0,"length":3,"stop_id":"StopArea:HMVE"},{"from":0,"length":3,"stop_id":"StopArea:ABDU"},{"from":0,"length":3,"stop_id":"StopArea:ADAP"}],"value1":0,"value2":3}

Petite coquille dans mon code je mappais sur stop_id. Par contre toujours pas possible de variabiliser la pagination limit stopId.from, stopId.length

^ ArangoError: syntax error, unexpected identifier near 'stopId.from, stopId.length ...' at position 26:17 (while parsing)

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://docs.arangodb.com/3.1/AQL/Operations/Limit.html

Note that variables and expressions can not be used for offset and count. Their values must be known at query compile time, which means that you can use number literals and bind parameters only.

:/

return new Promise((resolve, reject) => {
db().query(aql`
for stopId in ${stopIds}
let from = stopId.from
Copy link
Collaborator

Choose a reason for hiding this comment

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

Qu'est-ce ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

un reste de fail :) L'impression que je peux pas variabiliser les valeur de LIMIT dans AQL...

let stops = (
for stop in stops
filter stop.parent_station == ${stop_id}
filter stop.parent_station == stopId
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je crois que c'est un peu plus performant si tu fais filter stop.parent_station in ${stopIds} plutôt que le for.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alors oui mais en fait les paramètres de pagination font partie de ta clé et doivent pouvoir être différents pour chaque arrêt. Donc tu es obligé de revoir un peu ta requête et de faire un for.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Je suis obliger de wraper avec un 'for' ce qu'attend dataloader en retour est une Promise<array<value>> et l'array doit être de la même taille que le tableau passer en paramètre de const getRoutesByStationIds = (stopIds) => {

return route
)

return routes
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je ne suis pas sûr que cette requête marche pour plus d'un arrêt, parce que la variable routes est écrasée.

Copy link
Owner Author

Choose a reason for hiding this comment

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

La requête semble fonctionner comme il faut pourtant

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oui en fait AQL n'a que faire de l'indentation. Le for va jusqu'au return donc c'est OK.

}

const getRoutesByStationIds = (stopIds) => {
const { from, length } = stopIds[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il faudrait que les paramètres de pagination puissent différents pour chaque id. Mais j'imagine que c'est pour commencer là. :)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oui je sais pas trop comment les gérer pour le moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants