Bug #28175
Faire de order_by une liste plutôt qu'une chaîne
100%
Description
Pour permettre le tri sur plusieurs colonnes (et dans le future un tri déscendant).
Fichiers
Révisions associées
engine: make Dimension.order_by a list (fixes #28175)
Historique
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Fichier 0001-schema-automatically-cast-scalar-value-to-list-value.patch 0001-schema-automatically-cast-scalar-value-to-list-value.patch ajouté
- Fichier 0002-engine-make-Dimension.order_by-a-list-fixes-28175.patch 0002-engine-make-Dimension.order_by-a-list-fixes-28175.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Frédéric Péters il y a plus de 5 ans
Un rebasage sans doute a laissé deux "import contextlib" dans engine.py.
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Fichier 0001-schema-automatically-cast-scalar-value-to-list-value.patch 0001-schema-automatically-cast-scalar-value-to-list-value.patch ajouté
- Fichier 0002-engine-make-Dimension.order_by-a-list-fixes-28175.patch 0002-engine-make-Dimension.order_by-a-list-fixes-28175.patch ajouté
Corrigé.
Mis à jour par Emmanuel Cazenave il y a plus de 5 ans
Je nage complet donc je me concentre sur les fondamentaux : beaucoup de code et très peu de changement dans les tests alors que t'as fait passer des patchs pour rendre le machin testable, est-ce bien raisonnable ?
Louperais-je quelque chose ?
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
Emmanuel Cazenave a écrit :
Je nage complet donc je me concentre sur les fondamentaux : beaucoup de code et très peu de changement dans les tests alors que t'as fait passer des patchs pour rendre le machin testable, est-ce bien raisonnable ?
Louperais-je quelque chose ?
Effectivement le code demande quelques explications, initialement members() renvoyait simplement les valeurs extraites de la table associé à la première jointure d'une de dimension1 ce qui avait quelques limitations :
1. on ne pouvait pas avoir une dimension simple directement en colonne, ça devait forcément être une clé externe vers une table de mesure (il fallait minimum une jointure par dimension)
2. ça ne respectait pas la contrainte de la jointure (inner, left, right, outer) puisqu'on ajoutait pas la jointure dans l'expression
3. si la dimension dépendait d'une double jointure (scéma en flocon de neige ou "snowflake", ex.: formdata.formdef_id -> formdef.catagory_id -> category) par exemple pou son tri, l'expression de la jointure n'étant pas calculé on ne pouvait pas l'utiliser
4. dernière limitation, le tri se faisait forcément sur une colonne unique désigné soit par order_by
ou value
.
Ici je fais plusieurs chose:
1. au lieu de considérer que le FROM c'est forcément la première valeur dans join
je calcule une vrai jointure via build_table_expression()
ça m'a obligé à factoriser ce code et à y réintégrer le calcul du join_tree qui en fait aurait du en faire partie depuis le début (pour chaque jointure demandait par une dimension, j'ajoute ses dépendances en cascade, pour reprendre l'exemple plus haut, la catégorie nécessite de connaître le formdef, qui nécessite de connaître le formdata, 2 dépendances), si une dimension n'est qu'une simple colonne de la table de fait, maintenant ça fonctionne
2. pour obtenir la listes des membres je dois faire un GROUP BY
sur la valeur et le label de la dimension, seulement si en plus je trie par une ou plusieurs colonnes, je dois ajouter ces colonnes au GROUP BY
(toute colonne requété dans un group by doit apparaître dans le group by (c'est faux si le group by contient des clés primaires, mais comme on est pas capable de le détecter le mieux c'est de tout mettre)
Les modifications au schéma et aux test testent tout ça, même si ça parait court, la complexification d'order_by fait que la jointure devient nécessaire (sans elle l'ORDER BY foirerait et le test vérifie bien qu'on a un nouvel ordre qui dépend bien de l'ordre dans category et subcategory).
En attendant je constitue un jeu de test basé sur un export Grenoble, qui permettra d'avoir du vrai (je charge les données et les visus, et je vérifie que tout s'affiche bien, peut-être que je stockerai les résultats attendus pour voir quand ça bouge).
1 Voir la notion de schéma en étoile : https://fr.wikipedia.org/wiki/%C3%89toile_(mod%C3%A8le_de_donn%C3%A9es)
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Fichier 0001-schema-automatically-cast-scalar-value-to-list-value.patch 0001-schema-automatically-cast-scalar-value-to-list-value.patch ajouté
- Fichier 0002-engine-make-Dimension.order_by-a-list-fixes-28175.patch 0002-engine-make-Dimension.order_by-a-list-fixes-28175.patch ajouté
avec quelques ajustements par rapport au dernier test grandeur nature poussé.
Mis à jour par Emmanuel Cazenave il y a plus de 5 ans
- Statut changé de Solution proposée à Solution validée
Merci pour les explications, vas-y donc.
Mis à jour par Benjamin Dauvergne il y a plus de 5 ans
- Statut changé de Solution validée à Résolu (à déployer)
- % réalisé changé de 0 à 100
Appliqué par commit 0dce40e5e9caef37adc5c880dbacea0e3725e775.
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Statut changé de Résolu (à déployer) à Solution déployée
schema: automatically cast scalar value to list values (#28175)