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

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

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

landryb
Copy link
Contributor

@landryb landryb commented Jul 24, 2024

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 :)

…as l'identifiant majic (ccovoi) (3liz#345)

ces codes voies proviennent de l'import depuis TOPO
les valeurs sont quand même uniques en concaténant le code insee+ccoriv.
@landryb landryb marked this pull request as ready for review July 29, 2024 12:02
@landryb landryb mentioned this pull request Jul 29, 2024
@mdouchin mdouchin self-assigned this Jul 31, 2024
@mdouchin
Copy link
Collaborator

je bosse sur l'index...

@mdouchin
Copy link
Collaborator

@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
Copy link
Contributor Author

landryb commented Jul 31, 2024

sans index, cette requete (prise depuis #345 (comment)) 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
Copy link
Collaborator

mdouchin commented Aug 1, 2024

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

@mdouchin
Copy link
Collaborator

mdouchin commented Aug 1, 2024

@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
Copy link
Contributor Author

landryb commented Aug 1, 2024

@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
Copy link
Collaborator

mdouchin commented Aug 1, 2024

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
Copy link
Contributor Author

landryb commented Aug 1, 2024

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
Copy link
Collaborator

mdouchin commented Aug 1, 2024

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
Copy link
Contributor Author

landryb commented Aug 1, 2024

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
Copy link
Collaborator

mdouchin commented Aug 1, 2024

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

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
Copy link
Collaborator

mdouchin commented Aug 2, 2024

Je relance un import pour tester

@mdouchin
Copy link
Collaborator

mdouchin commented Aug 2, 2024

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
Copy link
Collaborator

mdouchin commented Aug 2, 2024

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
Copy link
Collaborator

mdouchin commented Aug 2, 2024

@landryb @MaelREBOUX Je viens de pousser un commit 8e82cd8 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

…ocal00 (substr de voie) & modification requêtes liées à la voie
@mdouchin
Copy link
Collaborator

mdouchin commented Aug 2, 2024

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
Copy link
Contributor Author

landryb commented Aug 5, 2024

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
Copy link
Contributor Author

landryb commented Aug 5, 2024

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
Copy link
Member

Gustry commented Aug 5, 2024

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
Copy link
Collaborator

mdouchin commented Aug 6, 2024

@MaelREBOUX as-tu eu le temps de tester ?

@MaelREBOUX
Copy link
Collaborator

@mdouchin je regarde

@MaelREBOUX
Copy link
Collaborator

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
Copy link
Contributor Author

landryb commented Aug 7, 2024

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 #345 (comment) pour debug-print les requetes SQL faites ;)

@mdouchin
Copy link
Collaborator

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
Copy link
Contributor Author

landryb commented Aug 19, 2024

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
Copy link
Member

Gustry commented Aug 19, 2024

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 ;-)

@Gustry Gustry merged commit efb9087 into 3liz:master Aug 19, 2024
2 checks passed
rldhont added a commit to 3liz/lizmap-cadastre-module that referenced this pull request Aug 27, 2024
rldhont added a commit to 3liz/lizmap-cadastre-module that referenced this pull request Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants