search

getodk / collect

ODK Collect is an Android app for filling out forms. It's been used to collect billions of data points in challenging environments around the world. Contribute and make the world a better place! ✨📋✨
https://docs.getodk.org/collect-intro
Other
718 stars 1.38k forks source link

Don't create a search/pulldata database when pulldata is used over an Entity List #6471

Open lognaturel opened 3 weeks ago

lognaturel commented 3 weeks ago

Following https://github.com/getodk/collect/pull/6451, pulldata over an Entity List uses the local Entity List database. However, the corresponding search/pulldata database is still being created. This increases form open time for users and leads to side effects like https://github.com/getodk/collect/issues/6461

Currently search() does not use the Entity List database so knowing that a source CSV is for an Entity List is not a sufficient signal to skip search/pulldata database creation.

This could be addressed as part of https://github.com/getodk/collect/issues/6454 or on its own.

seadowg commented 2 days ago

I'm thinking we should fix this by skipping the creation of the dynamic preload data DB if a form meets the follow two conditions:

The big change we'll need to make for that is to persist the "flag" that an instance (media file) is an entity list so that can be determined when parsing a form. There are a lot of other reasons that storing details about media files could be useful though, so I think that's a good change to make here.

Very related to this is how we then deal with search which, as @lognaturel points out, does not currently support local entities. I'm thinking we can approach it like this:

@lognaturel what are your thoughts on this plan? I think it's good to have a long term plan so we know the context surrounding fixing this issue.

lognaturel commented 1 day ago

The short-term plan sounds good and I agree that we will need to know whether a form attachment is an Entity List at a time other than download for other reasons, most notably for https://github.com/getodk/collect/issues/6425. I think that issue is higher priority than this one so maybe it can drive out storing that state.

Introduce a new XLSForm sugar to eventually replace search that pyxform translates to a normal instance('blah)...` expression

I think we already have this: select_one_from_file with a choice filter. And I think it has largely replaced search already because it's so much more comfortable to use. search is much less common than pulldata.

seadowg commented 1 day ago

The short-term plan sounds good and I agree that we will need to know whether a form attachment is an Entity List at a time other than download for other reasons, most notably for https://github.com/getodk/collect/issues/6425. I think that issue is higher priority than this one so maybe it can drive out storing that state.

Agreed!

I think we already have this: select_one_from_file with a choice filter. And I think it has largely replaced search already because it's so much more comfortable to use. search is much less common than pulldata.

Great point! I was forgetting that choice_filter already removes the need to deal with instance('blah')/root/item. There are other places where you do need to resort to that syntax, but search wouldn't help you there anyway.

What are your thoughts on deprecating search?