correction de la recherche sur adresse avec des codes voies n'ayant pas l'identifiant majic (ccovoi)

landryb commented 3 months ago

ces codes voies proviennent de l'import depuis un FANTOIR généré depuis TOPO, les valeurs sont quand même uniques en concaténant le code insee+ccoriv.

en attendant que #345 soit 'vraiment' corrigé par un import direct de TOPO :)

mdouchin commented 3 months ago

je bosse sur l'index...

mdouchin commented 3 months ago

@landryb j'ai rajouté un commit pour l'index. Pourrais tu tester stp ? Notamment sur des gros jeux de données (avec et sans) si tu as le temps ?

landryb commented 3 months ago

sans index, cette requete (prise depuis https://github.com/3liz/QgisCadastrePlugin/issues/345#issuecomment-2239043379) me renvoie 5 records après 2mn40:

SELECT ogc_fid, tex, idu, geo_section, geom, comptecommunal, geo_parcelle
     FROM "qad2024"."parcelle_info" WHERE 2>1 AND SUBSTR(voie,0,7)||SUBSTR(voie,12,4) = '150003B071' ORDER BY geo_parcelle;
Time: 161498.548 ms (02:41.499)

parcelle_info contient 14802390 enregistrements - d'ailleurs je vois que tu fais l'index sur la table parcelle, mais a priori c'est parcelle_info qui est requeté par le plugin ?

si j'essaie de créer l'index avec ta requete sql (sur la table parcelle_info), psql me jette:

CREATE INDEX idx_parcelle_info_voie_substr ON qad2024.parcelle_info (SUBSTR(voie,0,7) || SUBSTR(voie,12,4));
ERROR:  syntax error at or near "||"
LINE 1: ...substr ON qad2024.parcelle_info (SUBSTR(voie,0,7) || SUBSTR(...

il faut a priori une paire de parentheses en plus, cf https://www.postgresql.org/message-id/37d451f704102020346fdbb855%40mail.gmail.com ?

# CREATE INDEX idx_parcelle_info_voie_substr ON qad2024.parcelle_info ((SUBSTR(voie,0,7) || SUBSTR(voie,12,4)));
CREATE INDEX
Time: 29276.426 ms (00:29.276)
# \di+ qad2024.idx_parcelle_info_voie_substr
                                         List of relations
 Schema  |             Name              | Type  |  Owner   |     Table     |  Size  | Description 
---------+-------------------------------+-------+----------+---------------+--------+-------------
 qad2024 | idx_parcelle_info_voie_substr | index | qadastre | parcelle_info | 446 MB |

et la meme requete sql qu'au début me renvoie les 5 meme records en Time: 70.289 ms, donc l'idée d'avoir un index est la bonne, il faut juste vérifier sur quelle table(s?)...

mdouchin commented 3 months ago

Merci @landryb en effet, c'est parcelle_info ! Je vais corriger. Merci beaucoup pour le test, l'index aide bien en effet :+1:

mdouchin commented 3 months ago

@landryb J'ai repoussé et changé l'endroit où je crée l'index:

l'index sur des expressions n'est pas compatible dans Sqlite, donc on ne doit pas l'ajouter si on n'est pas dans une base PostgreSQL
il est donc ajouté par une méthode Python pendant l'import (à la fin de l'import).

Seul souci auquel je n'avais pas pensé, dans le cas d'imports successifs, on va supprimer et recréer les indexes ajoutés avec cette méthode endImport. Je ne pense pas que ce soit trop couteux, donc on va le garder comme cela si ça te va.

landryb commented 3 months ago

@landryb J'ai repoussé et changé l'endroit où je crée l'index:
* l'index sur des expressions n'est pas compatible dans Sqlite, donc on ne doit pas l'ajouter si on n'est pas dans une base PostgreSQL

* il est donc ajouté par une méthode Python pendant l'import (à la fin de l'import).
Seul souci auquel je n'avais pas pensé, dans le cas d'imports successifs, on va supprimer et recréer les indexes ajoutés avec cette méthode endImport. Je ne pense pas que ce soit trop couteux, donc on va le garder comme cela si ça te va.

ok, dans mon cas il sera crée 12 fois, mais bon ... je vois que c'est effectivement plus logique de le mettre au meme endroit que la création des autres indexes.

@MaelREBOUX doit tester ce que ca donne chez lui avec/sans l'index .. une fois validé il me semble qu'on sera bons pour merger (et releaser ;)

mdouchin commented 3 months ago

bon, je viens de tester sur une bdd sqlite via

sqlite3 /tmp/test.sqlite

sqlite> CREATE TABLE test (id integer, nom text);

sqlite> PRAGMA table_info('test');
0|id|INTEGER|0||0
1|nom|TEXT|0||0

sqlite> CREATE INDEX idx ON test (nom);
# no souci

sqlite> CREATE INDEX idx_substr ON test ((substr(nom, 2, 14)||substr(nom, 3, 5)));
# no souci non plus

La création d'index avec le substr a fonctionné sans erreur. Je dirais tant pis, on va considérer que les données dans une base SQLite pour le cadastre sont peu volumineuses.

landryb commented 3 months ago

Je dirais tant pis, on va considérer que les données dans une base SQLite pour le cadastre sont peu volumineuses.

j'aimerais être 100% d'accord avec ca, mais j'ai un peu peur que la réalité soit autre.. je pense que pour beaucoup d'utilisateurs du plugin, 'postgis' veut dire holala c'est compliqué il faut un serveur et une base de données et par conséquent importent un département entier (voire plus..) dans du spatialite (je suis quasi certain d'avoir eu X demandes d'aide a ce sujet dans mes mails...). le résultat est probablement abominablement lent a l'utilisation, mais c'est self-contained et ne demande pas d'interagir avec un informagicien....

mais oui, sur le fond, on est d'accord que postgis c'est mieux dès qu'on dépasse qqs communes/un epci.

mdouchin commented 3 months ago

D'accord avec toi @landryb : il y a l'idéal, et ce que les utilisateurs font vraiment en fonction de leurs compétences, du temps qu'ils ont, du contexte.

Je vais pousser les tests Spatialite, et voir si cela fonctionnerait :D

Au sujet des indexes, je me souviens d'avoir choisi de les supprimer avant import puis les recréer après import pour améliorer les performances des commandes COPY/INSERT/UPDATE lancées par l'import. Mais je n'ai pas auditer dans le cas de 12 départements.. Et c'est à vérifier (qui de la table parcelle_info)

Peut-être pourrais tu adapter cette partie là pour ton fork, et ne lancer la création de tous les indexes à la main qu'à la fin ? Pour lister tous les indexes, il y a ceci qui peut aider :

SELECT * 
FROM pg_indexes 
WHERE schemaname = 'NOM_DU_SCHEMA' 
ORDER BY schemaname, tablename, indexname
;

landryb commented 3 months ago

nb: on a peut-etre besoin de 2 indexes de plus pour les MV qu'on genere dans cadastrapp, sur les tables local00 et voie, pour accélérer la jointure de georchestra/cadastrapp#743 faite sur l.ccodep||l.ccodir||l.ccocom||l.ccoriv=v.ccodep||v.ccodir||v.ccocom||v.ccoriv - @MaelREBOUX est en train de tester et pour lui la création des 2 vues matérialisées proprietebatie et proprietenonbatie etait très lente (pour ma part, je n'ai pas fait particulièrement attention au temps pris pour la création de ces 2 MV en comparaison de l'an dernier)

mdouchin commented 3 months ago

Je viens de constater un truc, on fait une jointure entre parcelle, geo_parcelle et voie pour la table parcelle_info :

Avec MAJIC https://github.com/3liz/QgisCadastrePlugin/blob/master/cadastre/scripts/plugin/edigeo_create_table_parcelle_info_majic.sql#L92
Sans MAJIC : pas fait (mais on pourrait ?)

Il va falloir regarder si la jointure entre la table parcelle et voie, toutes les 2 issues du MAJIC est bien opérante, cad si les objets de parcelle_info ont bien des adresses référencées dans les champs

CF champs utilisés avec un fallback sur les champs des autres tables, cf https://github.com/3liz/QgisCadastrePlugin/blob/master/cadastre/scripts/plugin/edigeo_create_table_parcelle_info_majic.sql#L47

CASE
        WHEN v.libvoi IS NOT NULL THEN trim(ltrim(p.dnvoiri, '0') || ' ' || trim(v.natvoi) || ' ' || v.libvoi)
        ELSE ltrim(p.cconvo, '0') || p.dvoilib
END AS adresse,

mdouchin commented 3 months ago

Je relance un import pour tester

mdouchin commented 3 months ago

hum, d'ailleurs ce n'est pas comme en python, le substr de PostgreSQL est comme ses tableaux : le premier index est 1, pas 0 -> je vais proposer amélioration de ce PR cf https://www.postgresql.org/docs/15/functions-string.html

mdouchin commented 3 months ago

bon, j'ai travaillé sur le plugin avec des modifications

modif du substr pour commencer à 0
modif de la requête de création de parcelle_info avec MAJIC
ajout de l'index déplacé de la méthode endImport de cadastre_import.py vers le SQL de création de parcelle_info

mdouchin commented 3 months ago

@landryb @MaelREBOUX Je viens de pousser un ~~commit 8e82cd8ac27cdc81ea16e7fa89094126222a517f~~ commit 0da1a56 qui écrase le précédent. Pas mal de requêtes étaient en fait impactées par ce changement dans le plugin Cadastre. Je vous laisse regarder dans CadastreApp

J'ai ajouté un index spécifique aussi sur local00 et parcelle en plus de celui sur parcelle_info

Pourriez-vous svp re-tester ? Je teste aussi de mon côté avec PostgreSQL et avec SQLite

mdouchin commented 3 months ago

Pourriez-vous svp re-tester ? Je teste aussi de mon côté avec PostgreSQL et avec SQLite

Je viens de tester sans souci avec PostgreSQL et SQLITE :

recherche par adresse OK
fiche parcellaire OK pour les différents onglets
export PDF OK

landryb commented 3 months ago

hum, d'ailleurs ce n'est pas comme en python, le substr de PostgreSQL est comme ses tableaux : le premier index est 1, pas 0 -> je vais proposer amélioration de ce PR cf https://www.postgresql.org/docs/15/functions-string.html

hmmm, je m'y perds toujours avec ca.. de toute facon je vois que tes tests fonctionnels sont concluants, et je viens de vérifier ca revient au même, eg 6 caracteres en partant du premier = 7 caracteres en partant du 0e, substr(voie,0,7) me donne la meme chose que substr(voie, 1,6).

je vois aussi que tu fais finalement aussi ces indexes pour la variante spatialite, c'est a mon avis pas plus mal :)

et il y'avait effectivement bien plus de requetes impactées, j'avais pas du bien chercher...

j'y pense, vu que depuis #453 on pointe vers le pseudo-fantoir 2024, peut-etre qu'il faudra aussi spécifier dans le readme que la nouvelle version du plugin est nécessaire pour que la recherche par voie fonctionne avec cette source de voies ?

landryb commented 3 months ago

j'ai fait un test rapide avec postgis sur la commune 43006 (ALLY dans la haute loire):

les 3 index avec substr dans le nom sont bien crées
la table parcelle_info a bien des valeurs dans le champ voie donc la jointure se fait bien
faire une recherche par adresse sur une des 329 valeurs de select distinct(adresse) from parcelle_info me renvoie bien a chaque fois un nombre de parcelles correspondant aux parcelles de cette voie/lieu-dit
j'ai bien l'adresse dans l'onglet résumé de la fiche d'info parcellaire
j'ai bien l'adresse dans la 2e page du relevé parcellaire
les 2 derniers tests ont été fait sur des parcelles remontées comme baties, et/ou non baties.

donc pour moi on est bons. @MaelREBOUX tu testes chez toi ?

Gustry commented 3 months ago

on pointe vers le pseudo-fantoir 2024, peut-etre qu'il faudra aussi spécifier dans le readme que la nouvelle version du plugin est nécessaire

Le millésime 2024 ne sera disponible que dans la prochaine version, donc je pense que c'est bon. Les gens se poseront la question de faire la MAJ.

mdouchin commented 3 months ago

@MaelREBOUX as-tu eu le temps de tester ?

MaelREBOUX commented 3 months ago

@mdouchin je regarde

MaelREBOUX commented 3 months ago

Bonjour,

J'ai tout remouliné hier soir. Je vois bien les indexes.

Les voies / adresses parraissent bien sur les formulaires. Par contre : je n'arrive pas à faire une recherche par adresse. Mais je n'utilise jamais le plugin donc je ne fais pê pas bien ? Je liste une voie mais il ne se passe rien ensuite.

landryb commented 3 months ago

Les voies / adresses parraissent bien sur les formulaires. Par contre : je n'arrive pas à faire une recherche par adresse. Mais je n'utilise jamais le plugin donc je ne fais pê pas bien ? Je liste une voie mais il ne se passe rien ensuite.

tu veux dire que tu selectionne une voie après qu'elle ait été autocomplétée avec ce que tu as tapé, et ca te donne 0 parcelle dans le champ au dessus ? si oui il te faut https://github.com/3liz/QgisCadastrePlugin/pull/451/files#diff-55aca78054c0f1d10a664afdc826f28c6e43c32d9054601e0d20179d9e8f686cL860 dans le plugin pour que la recherche remonte des parcelles.

et je rappelle https://github.com/3liz/QgisCadastrePlugin/issues/345#issuecomment-2239156667 pour debug-print les requetes SQL faites ;)

mdouchin commented 2 months ago

Je propose de faire le merge et de publier rapidement une version ? Au pire, si @MaelREBOUX voit des soucis, on sortira rapidement une nouvelle version de correction ?

landryb commented 2 months ago

Je propose de faire le merge et de publier rapidement une version ? Au pire, si @MaelREBOUX voit des soucis, on sortira rapidement une nouvelle version de correction ?

je suis d'accord, étant donné qu'on m'a déjà demandé 'quand sort la version avec le support de 2024' plusieurs fois.. :)

Gustry commented 2 months ago

je suis d'accord, étant donné qu'on m'a déjà demandé 'quand sort la version avec le support de 2024' plusieurs fois.. :)

Idem ;-)

3liz / QgisCadastrePlugin

correction de la recherche sur adresse avec des codes voies n'ayant pas l'identifiant majic (ccovoi) #451