SDFIdk / FIRE

🔥 FIRE - FIkspunktREgister
https://sdfidk.github.io/FIRE/
MIT License
4 stars 8 forks source link

Fix bug i hent_punkt #721

Closed krebslw closed 6 months ago

krebslw commented 8 months ago

Resolves #720

krebslw commented 8 months ago

Ja havde godt tænkt at det skulle testes. Og tror også dit foreslag vil virke fint. Synes det tilføjer noget "implicit" logik da man så antager at det korteste navn er det rigtige. Alternativt kan man sortere således:

SELECT * FROM punktinfo pi 
WHERE pi.infotypeid=347 AND pi.tekst LIKE '%SAND%' 
ORDER BY (CASE WHEN pi.tekst='SAND' THEN 1 ELSE 2 END);

hvis det altså kan lade sig gøre sqlalchemy-style (har lige tjekket at det virker med oracle.)

Det leder også til spørgsmålet om hvilken løsning der er hurtigst, jf. issue #422, skulle vi ikke gøre det unødigt langsomt.

kbevers commented 8 months ago

Synes det tilføjer noget "implicit" logik da man så antager at det korteste navn er det rigtige.

Vi laver følgende opslag

https://github.com/SDFIdk/FIRE/blob/619af08e91da4cfcb9f36da1929f821ab5256da3/fire/api/firedb/hent.py#L91-L109

hvor der matches på tre ting: ident, FO {ident} og GL {ident}. Det skulle meget gerne resultere i at det korteste er det eksakte match. Hvis indholdet af ident har et FO- eller GL-præfix kan det kun matche på den første. Så skulle man ende i den uheldige situation at man leder efter FO SAND, så kan man skrive det eksplicit og med sikkerhed fremfinde det.

Det leder også til spørgsmålet om hvilken løsning der er hurtigst, jf. issue https://github.com/SDFIdk/FIRE/issues/422, skulle vi ikke gøre det unødigt langsomt.

Det der er dyrt i hent_punkt og venner, er at lave mappingen mellem pythonobjekter og SQL-udtryk. Altså alle de joins der skal laves på kryds og tværs for at udfylde alle felter i Punkt. Jeg tror ikke en sortering på databasesiden koster noget nævneværdigt i den sammenhæng, men det skal times for at kunne afgøres med sikkerhed. Det er i hvert fald ikke noget jeg er synderligt bekymret for og jeg regner med en performance forbedring når vi skifter til SQLAlchemy 2.0 som tager hånd om en del af den sløvhed vi ser nu.

kbevers commented 6 months ago

@krebslw Som du kan se har jeg tilføjet en test og backportet til 1.6. Vi fik aldrig lejlighed til at lave testen sammen og nu har jeg lige brug for at dit fix kommer i spil. Brug gerne lidt tid på at læse mine tilføjelser igennem, så du har en ide om hvordan noget lignende kan gøres i fremtiden.