fledge-iot / fledge

An open source platform for the Industrial Internet of Things, it acts as an edge gateway between sensor devices and cloud storage systems.
https://www.lfedge.org/projects/fledge/
Apache License 2.0
117 stars 43 forks source link

FOGL-8065: mostrecent parameter fix for sqlite storage engines #1367

Closed pintomax closed 1 month ago

pintomax commented 2 months ago

FOGL-8065: mostrecent parameter fix for sqlite storage engines

pintomax commented 1 month ago

@ashish-jabble please provide examples of the new payload being passed to storage.

The fix here is to pass some extra data to date string so SQLite query fetches the upper timestamp.

It there are other scenarios similar to this, then we can then try a different solution for SQLite only

MarkRiddoch commented 1 month ago

We should address/fix the real problem, this is pure hack made on API side.

Sorry, in what way is this not addressing the real problem?

This is the one of use case for the lost reading given in JIRA, we have more scenarios coming; there is another limitation on storage side to not having support of Between operator which means we have another workaround implemented at API side for most recent functionality to pass datetime UTC string in payload as newer/older KV pair work with datetime.

Why do we need a between operator to fix this particular problem, there is not set of timestamps in the API to require a between.

ashish-jabble commented 1 month ago

Sorry, in what way is this not addressing the real problem?

In JIRA, I have given the payloads which actually hit the storage readings query endpoint.

Sending user_ts in datetime UTC string with micorseconds, e.g. 2024-05-17 10:01:32.702005

This works with PostgreSQL but not the SQLite.

Do We know why?

My expectation was to see a fix at database layer for SQLite plugins, not extending the datetime string. Do We store differently or comparison algorithm is broken with conversion mechanism?

Coming to given fix, by adding "0001" to upper limit date string; What is the explanation of this?

Why do we need a between operator to fix this particular problem, there is not set of timestamps in the API to require a between.

I didnot comment for this problem, But the fact that we had similar situations for between operator and we ended up adding a hack on API side as a workaround, instead of adding proper supports at Storage layer.

Imagine end users writing their own APIs.

MarkRiddoch commented 1 month ago

I am pulling this. This is a very simple, minor issue that we have spent far too long over. Closing the pull request and looking at abandoning the functionality ion the product.