Project

General

Profile

Development #38067

Revoir complètement la génération des tableaux et la gestion des valeurs NULL dues aux OUTER JOIN

Added by Benjamin Dauvergne 18 days ago. Updated 12 days ago.

Status:
Solution déployée
Priority:
Normal
Target version:
-
Start date:
28 Nov 2019
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Pour chaque type de dimension on peut définir un label d'absence.

L'objectif initial ne permettait pas d'avoir des tableaux sensés :
  • voir tous les éléments des dimensions dans tous les cas
  • avoir la valeur Aucun(e) dans tous les cas où une dimension peut-être absente (ici c'est détecté à la volée en fonction des données)

J'en ai profité pour remettre à plat la façon dont les résultats des requêtes sont produits (on conserve les objets Dimension et Measure tout du long désormais plutôt que de passer par des dicos mal définis).

0001-schemas-add-an-absent_label-property-to-Dimension-38.patch View (1.86 KB) Benjamin Dauvergne, 28 Nov 2019 09:29 PM

0004-tests-keep-new-table-contents-during-tests-38067.patch View (1.25 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0003-tests-reformat-json-files-for-easier-update-38067.patch View (6.97 MB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0008-engine-cache-proxy-descriptor-result-on-engine-s-cub.patch View (930 Bytes) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0006-engine-cache-EngineDimension.members-38067.patch View (3.15 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0005-utils-cache-get_warehouses-result-based-on-schema-fi.patch View (1.67 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0009-engine-hide-NULL-values-of-json-dimensions-in-inline.patch View (1.26 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0010-engine-use-LEFT-OUTER-JOIN-to-join-with-inline-json-.patch View (709 Bytes) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0011-schemas-add-an-absent_label-property-to-Dimension-38.patch View (4.69 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0012-visualization-utils-remove-dead-import-38067.patch View (1.15 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0007-engine-do-not-hardcode-cube-s-json-field-name-38067.patch View (995 Bytes) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0001-tests-load-schema2-fixture-only-one-time-38067.patch View (2.43 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0013-overhaul-of-query-to-table-transformation-38067.patch View (304 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0002-tests-use-tabulate-to-compare-tables-38067.patch View (1.87 KB) Benjamin Dauvergne, 03 Dec 2019 01:53 PM

0004-tests-keep-new-table-contents-during-tests-38067.patch View (1.25 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0003-tests-reformat-json-files-for-easier-update-38067.patch View (6.97 MB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0008-engine-cache-proxy-descriptor-result-on-engine-s-cub.patch View (930 Bytes) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0006-engine-cache-EngineDimension.members-38067.patch View (3.15 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0005-utils-cache-get_warehouses-result-based-on-schema-fi.patch View (1.67 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0009-engine-hide-NULL-values-of-json-dimensions-in-inline.patch View (1.26 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0010-engine-use-LEFT-OUTER-JOIN-to-join-with-inline-json-.patch View (709 Bytes) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0011-schemas-add-an-absent_label-property-to-Dimension-38.patch View (4.69 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0012-visualization-utils-remove-dead-import-38067.patch View (1.15 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0007-engine-do-not-hardcode-cube-s-json-field-name-38067.patch View (995 Bytes) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0001-tests-load-schema2-fixture-only-one-time-38067.patch View (2.43 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0013-overhaul-of-query-to-table-transformation-38067.patch View (304 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0002-tests-use-tabulate-to-compare-tables-38067.patch View (1.87 KB) Benjamin Dauvergne, 03 Dec 2019 03:11 PM

0001-visualization-views-in-JSON-fix-0-dimension-case-380.patch View (968 Bytes) Benjamin Dauvergne, 03 Dec 2019 03:59 PM


Related issues

Related to OLAP / Businesse Intelligence pour Publik - Development #38066: utiliser uniquement LEFT OUTER JOIN pour les jointures avec les tables d'item Solution déployée 28 Nov 2019
Duplicated by BiJoe - Bug #38052: KeyError at /visualization/45/json/ (u'None', u"Grand Chamb\xe9ry Cours d'eaux") Fermé 28 Nov 2019

Associated revisions

Revision 7f4f373e (diff)
Added by Benjamin Dauvergne 13 days ago

tests: load schema2 fixture only one time (#38067)

Use special db fixture (using pytest-django primitives and
transaction.atomic()) to initialize bijoe visualization only one time
for all tests on the schema2 fixture. It improves tests run time.

Revision 0a612f24 (diff)
Added by Benjamin Dauvergne 13 days ago

tests: use tabulate to compare tables (#38067)

Comparing strings with assert gives better diffs thant comparing list of
lists when using the pytest's `-vv` option.

Revision 65a78b93 (diff)
Added by Benjamin Dauvergne 13 days ago

tests: reformat json files for easier update (#38067)

Add script format_json_fixtures.py to canonicalize every JSON files in
fixtures to simplify management of changes in visualization results.

Revision 9cc619ee (diff)
Added by Benjamin Dauvergne 13 days ago

tests: add option to update tables during tests (#38067)

If table contents change, we can measure the change and eventually
accept by copying new_table.json to tables.json. To activate it, use
--bijoe-store-table .

Revision 33a8eabd (diff)
Added by Benjamin Dauvergne 13 days ago

utils: cache get_warehouses() result based on schema files paths (#38067)

Revision a868f11d (diff)
Added by Benjamin Dauvergne 13 days ago

engine: cache EngineDimension.members (#38067)

Revision 3f7bedf0 (diff)
Added by Benjamin Dauvergne 13 days ago

engine: do not hardcode cube's json field name (#38067)

Revision 47015684 (diff)
Added by Benjamin Dauvergne 13 days ago

engine: cache proxy descriptor result on engine's cubes (#38067)

Revision d603f148 (diff)
Added by Benjamin Dauvergne 13 days ago

engine: hide NULL values of json dimensions in inline join table (#38067)

Revision 2422edd7 (diff)
Added by Benjamin Dauvergne 13 days ago

engine: use LEFT OUTER JOIN to join with inline json join table (#38067)

Revision e8e1d573 (diff)
Added by Benjamin Dauvergne 13 days ago

schemas: add an absent_label property to Dimension (#38067)

Also force lazy strings to unicode in to_json().

Revision 6402c7ea (diff)
Added by Benjamin Dauvergne 13 days ago

visualization/utils: remove dead import (#38067)

Revision 72e8d4c8 (diff)
Added by Benjamin Dauvergne 13 days ago

overhaul of query to table transformation (#38067)

  • during query obtain dimension id and label if a different projection
    is defined
  • use object to materialize query results : Cells, DimensionCell,
    MeasureCell
  • handle stringification in the *Cell classes
  • never ignore NULL dimension's values (it's detected in
    Visualization.data() and added to the list of dimension members)
  • sum of columns is only computed if there are more than one column
  • sum of rows is only computed if there are more than one row
  • full sum is only computed if there are more thant one column and more
    than one row
  • 1 dimension table are computed in the same maner as 2 dimensions
    tables, no more discrepancies
  • JSON web-service now use the same base methods table_2d() and table_1d()
    as the native rendering in bijoe
  • EngineDimension.members is specialized for bool dimensions (as it's
    always True/False)

Revision 10b4405f (diff)
Added by Benjamin Dauvergne 12 days ago

engine: propagate filters to dimension's members enumeration (#38067)

Revision 980cb76a (diff)
Added by Benjamin Dauvergne 12 days ago

settings: improve tests runtime by turning DEBUG off (#38067)

History

#1 Updated by Benjamin Dauvergne 18 days ago

#2 Updated by Benjamin Dauvergne 16 days ago

  • Subject changed from Définir une valeur d'absence pour une dimension to Définir un label d'absence pour une dimension
  • Description updated (diff)

#5 Updated by Benjamin Dauvergne 13 days ago

  • Description updated (diff)
  • Subject changed from Définir un label d'absence pour une dimension to Revoir complètement la génération des tableaux et des valeurs NULL due aux OUTER JOIN

#6 Updated by Frédéric Péters 13 days ago

format_json_fixtures.py, dans le json.dump, j'ajouterais separators=(',', ': '), pour ne pas avoir d'espace en fin de ligne. (ou je forcerais à exécuter le script avec Python >= 3.4, où c'est devenu le paramétrage par défaut.

return unicode(self.label); ce serait bien de prendre en compte Python 3 dans le nouveau code.

  File "/home/fred/src/eo/bijoe/bijoe/visualization/views.py", line 311, in get
    data = cell_value(visualization.data()[0][0])
  File "/home/fred/src/eo/bijoe/bijoe/visualization/views.py", line 281, in cell_value
    if cell.measure.type == 'duration' and cell.value is not None:
AttributeError: 'list' object has no attribute 'measure'

Sur une visualisation avec juste "nombre de demandes", pas de regroupements/répétitions, rien. (cell vaut [])

Sur le fond, je ne connais pas assez pour relire, mais ça m'irait d'avoir une petite description de ce qui a été entrepris dans ces patchs. (la description revue contient déjà un peu plus d'infos).

#8 Updated by Benjamin Dauvergne 13 days ago

  • Related to Development #38066: utiliser uniquement LEFT OUTER JOIN pour les jointures avec les tables d'item added

#10 Updated by Benjamin Dauvergne 13 days ago

Frédéric Péters a écrit :

format_json_fixtures.py, dans le json.dump, j'ajouterais separators=(',', ': '), pour ne pas avoir d'espace en fin de ligne. (ou je forcerais à exécuter le script avec Python >= 3.4, où c'est devenu le paramétrage par défaut.

Ok.

return unicode(self.label); ce serait bien de prendre en compte Python 3 dans le nouveau code.

Je me suis dit ça mais comme il y a déjà beaucoup de unicode() dans les mêmes fichiers j'ai préféré garder ça comme ça pour tout faire d'une seule traite après plutôt que de mélanger portage python3 et ce ticket dont je savais déjà qu'il allait devenir bordélique.

[...]

Sur une visualisation avec juste "nombre de demandes", pas de regroupements/répétitions, rien. (cell vaut [])

Pourtant là ça marche :

https://statistiques-vincennes.test.entrouvert.org/visualization/warehouse/demarches_vincennes_test_au_quotidien_com/formdata_creche_renouvellement_de_contrat_2017/?drilldown_y=&drilldown_x=&filter__certif=1&filter__receipt_time_2=&measure=count&representation=table&csrfmiddlewaretoken=vg2ccQv7GQw2Q97hG3cyyjIlwP2GLHIzHmWqEKp4NvoTQidL4TetIhkNqvax7uxp&filter__receipt_time_0=&filter__receipt_time_1=&loop=

C'est en local ou tu l'as vu sur une instance de test ?

PS: j'ai compris, j'ai posé un patch au dessus du dernier commit qui corrige le souci je pense.

Sur le fond, je ne connais pas assez pour relire, mais ça m'irait d'avoir une petite description de ce qui a été entrepris dans ces patchs. (la description revue contient déjà un peu plus d'infos).

Je suis parti de mon constat sur #38031 : je pensais que ce que demandait Thibault était une évolution mais en fait on calcule très mal les axes des tableaux et de manière différente selon qu'on a deux axes ou un seul (avec deux axes on génère la liste des valeurs pour chaque dimension, dans le cas 1 dimension on prenait directement les valeurs retournées par la requête SQL).

Ça m'a d'abord fait ouvrir #38066, puis je me suis aperçu qu'agir sur le schéma généré par wcs-olap ne suffirait pas vu que le calcul des tables ne fonctionnait pas bien dans le cas pointé: les tables à deux dimensions ignore les NULL, les tables à 1 dimensions ignore les valeurs auxquelles ne correspondent aucun formulaire dans les deux cas c'est insuffisant; de plus le code coté vue JSON était différent augmentant les chances d'avoir des résultats différents entre bijoe et combo.

Maintentant je construis un defaultdict() "grid" dans table_2d et table_1d qui contient par défaut la mesure vide (en général 0), comme j'ai eu besoin de travailler directement sur la correspondance entre valeur de dimension et leur liste Member(id,label) il a fallu que je revienne au niveau de Engine.query() sur le fait de ne renvoyer que la valeur "value_label" pour chaque dimension, d'où l'ajout dans la génération SQL de ça :

-                projections.append('%s AS %s' % (dimension.value_label or dimension.value,
-                                                 dimension.name))
+                projections.append('%s AS %s' % (dimension.value, dimension.name + '_value'))
+                if dimension.value_label:
+                    projections.append('%s AS %s' % (dimension.value_label, dimension.name + '_label'))

aussi à plusieurs endroits (paramètre de Engine.query() ou son résultat) je passais soit le nom d'une dimension ou un extrait de ses attributs j'ai trouvé plus simple désormais de travailler directement avec les objets Measure/Dimension et de faire une différence entre les cellules pour les mesures et les dimensions.

Désormais dans cell.dimensions[*].value on a des choses qui correspondent à member.id renvoyé par EngineDimension.members() plutôt que de faire correspondre sur le label (ça ne marchait plus avec la valeur NULL mappé sur "Aucun(e)" faire autrement rendait le code encore plus illisible).

Avec le nouveau code on devrait avoir plusieurs :
  • un traitement correct des dimensions dans tous les cas (on ignore plus les NULL sauf si aucun décompte ne leur est lié, on ignore plus les valeurs non-NULL mais qui ont un décompte à 0 quand il y avait un left outer join)
  • on ne crée plus de colonne/ligne total si c'est inutile,
  • on a accès à la définition des dimensions/mesures tout du long de la chaîne de génération des données
  • les sorties en tableau, en graphique, en ODS et en JSON utilisent toute le même code
  • la transformation des valeurs en chaîne est gérée au niveau engine et pas visualisation ce qui rend le code plus clair à cet endroit (plus de stringified()).

Les premiers commits sur les tests c'est vraiment en passant pour accélérer un peu les tests parce que j'ai du les faire tourner beaucoup pour voir ce qui changeait en bien ou pas au fur et à mesure (on gagne 2 à 3 minutes sur les 16 minutes usuelles).

Les commits autour des colonnes JSON c'est parce que pas mal de tests ont pété dans schema2 car ce sont des visualisations sur le cube "Tous les formulaires" et ça remontait des valeurs NULL en trop (ou ça les oubliait) dans les dimensions.

Je voulais modifier le moins possible mais au fur à mesure j'ai du remonter jusqu'au niveau de la génération des requêtes pour remettre les choses à plat.

#12 Updated by Frédéric Péters 13 days ago

PS: j'ai compris, j'ai posé un patch au dessus du dernier commit qui corrige le souci je pense.

(oui c'était en testant la branche en local, et oui ça fonctionne désormais).

#13 Updated by Benjamin Dauvergne 13 days ago

Le dernier patch est rebasé dans le dernier commit de la branche et la plate-forme de test est à jour sur cette même branche, si personne ne signale de régression en recette je sui d'avis de pousser; après #38066.

#14 Updated by Benjamin Dauvergne 12 days ago

  • Subject changed from Revoir complètement la génération des tableaux et des valeurs NULL due aux OUTER JOIN to Revoir complètement la génération des tableaux et la gestion des valeurs NULL dues aux OUTER JOIN

#15 Updated by Benjamin Dauvergne 12 days ago

  • Description updated (diff)

#16 Updated by Frédéric Péters 12 days ago

Dans #37899 j'ai tapé un commit dans combo suite à une visualisation qui était :

  • regroupement vertical : formulaire
  • répétition : catégorie

qui produisait une série de tableaux, genre :

Cadre de vie
------------
Démarche 1             | 12
Démarche 2             | 14
Démarche 3             | 7

Signalement
-----------
Arbre                  | 8
Déchets                | 4

Après ce ticket, le résultat est :

Cadre de vie
------------
Arbre                  | 0
Déchets                | 0
Démarche 1             | 12
Démarche 2             | 14
Démarche 3             | 7

Signalement
-----------
Arbre                  | 8
Déchets                | 4
Démarche 1             | 0
Démarche 2             | 0
Démarche 3             | 0

Alors perso, j'étais de toute façon d'avis pour dégager "répétition", ce changement de comportement va juste mettre tout le monde d'accord là-dessus.

#17 Updated by Frédéric Péters 12 days ago

Il manque une référence au ticket dans "apply filters to loop dimension enumeration too" et il y une partie "perf" qui s'est glissée dans "engine: propagate filters to dimension's members enumeration" et pourrait être son propre commit; à part ça je n'aurai rien à dire.

#18 Updated by Benjamin Dauvergne 12 days ago

Frédéric Péters a écrit :

Il manque une référence au ticket dans "apply filters to loop dimension enumeration too" et il y une partie "perf" qui s'est glissée dans "engine: propagate filters to dimension's members enumeration" et pourrait être son propre commit; à part ça je n'aurai rien à dire.

Corrigé ces deux soucis, j'ai refait une passer sur les différences dans tests/fixtures/schema2/tables.json et toutes les modifications sont celles souhaitées.

#19 Updated by Frédéric Péters 12 days ago

  • Status changed from Solution proposée to Résolu (à déployer)

Et tu as poussé dans master.

#20 Updated by Benjamin Dauvergne 12 days ago

commit 980cb76aaea2fee09039e294e0ea1c992b8a5b6b
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Wed Dec 4 19:30:27 2019 +0100

    settings: improve tests runtime by turning DEBUG off (#38067)

commit 10b4405fd11aee06f8c87ae47346c235f5c54a54
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Wed Dec 4 12:53:16 2019 +0100

    engine: propagate filters to dimension's members enumeration (#38067)

commit 72e8d4c83e534a5c2d2c02aa9d4b0010c95e4097
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sat Nov 30 03:59:00 2019 +0100

    overhaul of query to table transformation (#38067)

    * during query obtain dimension id and label if a different projection
    is defined
    * use object to materialize query results : Cells, DimensionCell,
    MeasureCell
    * handle stringification in the *Cell classes
    * never ignore NULL dimension's values (it's detected in
    Visualization.data() and added to the list of dimension members)
    * sum of columns is only computed if there are more than one column
    * sum of rows is only computed if there are more than one row
    * full sum is only computed if there are more thant one column and more
    than one row
    * 1 dimension table are computed in the same maner as 2 dimensions
    tables, no more discrepancies
    * JSON web-service now use the same base methods table_2d() and table_1d()
    as the native rendering in bijoe
    * EngineDimension.members is specialized for bool dimensions (as it's
    always True/False)

commit 6402c7ea99fea8e31634dff6f6e9f17fc393e082
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sat Nov 30 03:57:21 2019 +0100

    visualization/utils: remove dead import (#38067)

commit e8e1d573c57a90c977d1b00239040ebb13319f11
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Thu Nov 28 20:14:16 2019 +0100

    schemas: add an absent_label property to Dimension (#38067)

    Also force lazy strings to unicode in to_json().

commit 2422edd73341ded8809451fb35b62b5294a02f21
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:31:25 2019 +0100

    engine: use LEFT OUTER JOIN to join with inline json join table (#38067)

commit d603f148607da2cde67ef50194be3993d9efe7fa
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:31:08 2019 +0100

    engine: hide NULL values of json dimensions in inline join table (#38067)

commit 47015684372edfe0a4e53bbe7d717e4bd47fe306
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:29:48 2019 +0100

    engine: cache proxy descriptor result on engine's cubes (#38067)

commit 3f7bedf0d317324eed7af0e32194455172448a64
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:29:04 2019 +0100

    engine: do not hardcode cube's json field name (#38067)

commit a868f11d42b31e38536671ef4b4dd9afd7928abd
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:27:22 2019 +0100

    engine: cache EngineDimension.members (#38067)

commit 33a8eabdba2afba00cdd44146e63889eacf3f704
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:24:35 2019 +0100

    utils: cache get_warehouses() result based on schema files paths (#38067)

commit 9cc619ee0d0b3257e77ed3beb6bb371bbf6a9f36
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sat Nov 30 03:53:09 2019 +0100

    tests: add option to update tables during tests (#38067)

    If table contents change, we can measure the change and eventually
    accept by copying new_table.json to tables.json. To activate it, use
    --bijoe-store-table .

commit 65a78b93dcfdd0bbb305d4674bda163ee8d2b6da
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 01:35:15 2019 +0100

    tests: reformat json files for easier update (#38067)

    Add script format_json_fixtures.py to canonicalize every JSON files in
    fixtures to simplify management of changes in visualization results.

commit 0a612f24aca40c403fc3a7964e44134cc156270c
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sat Nov 30 03:52:14 2019 +0100

    tests: use tabulate to compare tables (#38067)

    Comparing strings with assert gives better diffs thant comparing list of
    lists when using the pytest's `-vv` option.

commit 7f4f373e89e01acca12362aebca42ec8715736cc
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Fri Nov 29 15:26:08 2019 +0100

    tests: load schema2 fixture only one time (#38067)

    Use special db fixture (using pytest-django primitives and
    transaction.atomic()) to initialize bijoe visualization only one time
    for all tests on the schema2 fixture. It improves tests run time.

#21 Updated by Frédéric Péters 12 days ago

  • Status changed from Résolu (à déployer) to Solution déployée

#22 Updated by Benjamin Dauvergne 12 days ago

  • Duplicated by Bug #38052: KeyError at /visualization/45/json/ (u'None', u"Grand Chamb\xe9ry Cours d'eaux") added

Also available in: Atom PDF