dsync icon indicating copy to clipboard operation
dsync copied to clipboard

Filtering: better "read" fns

Open Wulf opened this issue 2 years ago • 7 comments

I just wanted to share my thoughts on the 'read/paginate' functions:

They are very arbitrary unless we introduce a way to filter the results.

In #103, I attempted to add filter() helper which allows us to generate arbitrary WHERE clauses; however, it currently only supports "equals"-based clauses, and not things like:

  • string search
  • date/time start/end/range
  • between/greater-than/less-than number,
  • etc; essentially the full list of SQL operations allowed in WHERE clauses.

This is a major turning point for dsync which has primarily been a code-generation tool as opposed to a query-builder. Deciding to add filter() is the only way functions like read() and paginate() make sense. However, we can only go so far with the filtering before we end up with a query builder 😅.

So here's the thing: do we continue with this effort?

Some more thoughts: we should consider generating cursor-based pagination functions and may also want to split read into: read_first and read_all (or similar-named functions).

Anyway, no big deal, hope all is well 🙌

Wulf avatar Nov 10 '23 01:11 Wulf

Using the read() function to fetch a record with a specific primary key is a good-enough use case, so perhaps another option is to remove the paginate() fn?

Wulf avatar Nov 10 '23 01:11 Wulf

Using the read() function to fetch a record with a specific primary key is a good-enough use case, so perhaps another option is to remove the paginate() fn?

i would likely be in favor of removing .paginate (and related things in #103), unless we make it explicit that the default paginate function is meant to paginate without a specific clause (table order). as for the other functions, i think it is reasonable to have some CRUD functions based on the identifier(s)

hasezoey avatar Nov 10 '23 15:11 hasezoey

Perhaps we can put them behind a feature flag and keep them in experimental/unstable state

Wulf avatar Nov 11 '23 22:11 Wulf

Perhaps we can put them behind a feature flag and keep them in experimental/unstable state

that would also be a option, should it be a default though? and if disabled, should the old code be generated?

hasezoey avatar Nov 12 '23 14:11 hasezoey

Yeah, let’s leave it disabled by default.

That is to say: in the current version, this would only generate the paginate fn if some experimental flag was set. If we want to stabilize parts of these experiments in the future, we’ll put them behind specific flags or add them to the default set, according to our decision

Wulf avatar Nov 12 '23 17:11 Wulf

That is to say: in the current version, this would only generate the paginate fn if some experimental flag was set. If we want to stabilize parts of these experiments in the future, we’ll put them behind specific flags or add them to the default set, according to our decision

so if i understand correctly, we will remove the .paginate function from non-flag builds?

hasezoey avatar Nov 12 '23 18:11 hasezoey

See #126 -- removed .paginate from default set and also introduced some experimental API behind advanced-queries

Wulf avatar Jan 15 '24 01:01 Wulf