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

Bug in Place Model #187

Open
rmayore opened this issue Dec 25, 2024 · 1 comment
Open

Bug in Place Model #187

rmayore opened this issue Dec 25, 2024 · 1 comment
Labels
Type: Bug Fix something that isn't working as intended

Comments

@rmayore
Copy link

rmayore commented Dec 25, 2024

Describe the bug
https://github.com/medic/cht-pipeline/blob/main/models/contacts/place.sql

The accepted way to get the contact of a place, is couchdb.doc #>> '{contact,_id}', rather than couchdb.doc->>'contact_id'. You can refer to go to any cht couchdb medic database and look at a place document... or look at the legacy contactview_metadata views.

Similarly, the correct way to get the place of a non-place contact is couchdb.doc #>> '{parent,_id}' rather than couchdb.doc->>'place_id'. Still I'm not sure why a place table would have a place_id field.... presumably place.uuid is the id of the place so what exactly is place.place_id? Is it meant to be the parent place of the place?

Anyways, as a consequence, for cht pipelines that use this repo as a base the place table has null place_id and contact_id fields. I've confirmed this in both local setups and the prod KE MOH database that uses cht sync pipeline. With those fields being null, the extra overhead to join the raw couchdb table again just to get the contact of a place is massive.

Image

To Reproduce

  1. Step 1
  2. Step 2

Expected behavior

Logs

Screenshots

Environment

  • Instance
  • Client platform: (eg: Windows, MacOS, Linux)

Additional context

@rmayore rmayore added the Type: Bug Fix something that isn't working as intended label Dec 25, 2024
@witash
Copy link
Contributor

witash commented Jan 13, 2025

@rmayore

for place_id some configurations have a separate field called place_id which is separate from the _id of the place document in couchdb. If present, it will be extracted from the document and indexed, but if absent, it will be NULL. This is fine, it is optional if the document _id is sufficient and prefferred to uniquely identify the place.

For contact_id, I'm not sure if there's any cht instance that has a separate contact_id field. But even if there is, I think you're right that doc->contact->>_id would generally be more useful.

we can maybe coalesce all the variations like in

in the meantime, joining back to the couchdb table shouldn't have massive overhead unless the result set is large. Do you have an example model that is not performant due to this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Fix something that isn't working as intended
Projects
None yet
Development

No branches or pull requests

2 participants