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

EPIC: lists Schema + Setup #356

Closed
12 tasks done
nelsonic opened this issue Apr 18, 2023 · 18 comments
Closed
12 tasks done

EPIC: lists Schema + Setup #356

nelsonic opened this issue Apr 18, 2023 · 18 comments
Assignees
Labels
discuss Share your constructive thoughts on how to make progress with this issue enhancement New feature or enhancement of existing functionality epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. priority-1 Highest priority issue. This is costing us money every minute that passes. technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@nelsonic
Copy link
Member

nelsonic commented Apr 18, 2023

We’ve gone long enough without creating lists.
In order to unlock the next level of usefulness, we are going to need lists.
This was briefly outlined in dwyl/app#271 but I'm opening this issue for implementation in the MVP. i.e. this is the basic first implementation. 👌

list requirements:

  • When a person first authenticates with the App, automatically create their list called “All”

    • ALL items are added to this list by default.
      Should the setting to automatically add ALL items to the “Everything List” should be configurable?
      person can create a new list (e.g. “Backlog”) and use that the place where all new items are stored.

      Note: list creation interface is still TBD. #helpwanted

  • lists should have the following fields:

    • id (Int auto increment PK) of the list used for lookups and routing but not displayed to the person
    • text (String) - the name of the list that will be displayed in the App
    • person_id (Int) of the person that created the list
    • status (Int) a numeric status applied to the list, see: https://github.com/dwyl/statuses
  • A list can have an arbitrary number of items (zero or more) via the list_items lookup table.

  • items can be removed from a list completely.
    Rather than adding an extra (status) field to the list_items table that will be null 99% of the time.
    We simply use the position field and a special value: 999999.999 AKA “nine nines”
    (a nod to the 99.9999999% High Availability
    But in our case we want number to be ridiculously high, 999999.999 rounds to a Million.
    We cannot use 99.9999999 because we expect many (most?) “Everything” lists to exceed 100.

  • So to “remove” an item from a list, simply insert a new row into list_items with position=999999.999

    Note: we do not expect any real lists to have a Million items and if they do we can revisit this.
    A list with a Million items would be totally unmanageable by a human on a screen …
    If we get to the point where lists are being used by machines, we will revisit this.

Ordering

  • The order of the list_items is represented by the position field, as per: Reordering items #145 (comment)
  • When the person performs a re-order operation this inserts a new row into the list_items table thus preserving the full history of the list,
    This will ensure that people can replay changes made to a list e.g. if they are working in a remote team and want to see what changes were made by a team member.
@nelsonic nelsonic added enhancement New feature or enhancement of existing functionality epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. technical A technical issue that requires understanding of the code, infrastructure or dependencies priority-1 Highest priority issue. This is costing us money every minute that passes. discuss Share your constructive thoughts on how to make progress with this issue labels Apr 18, 2023
@nelsonic nelsonic self-assigned this Apr 18, 2023
@nelsonic nelsonic pinned this issue Apr 18, 2023
@nelsonic nelsonic mentioned this issue Apr 19, 2023
3 tasks
@nelsonic
Copy link
Member Author

MVP ERD before lists:
mvp-erd-before-lists

Without the migrations and versions noise:
mvp-erd-before-lists

With lists and list_items
mvp-erd-with-lists

@nelsonic
Copy link
Member Author

nelsonic commented Aug 7, 2023

Need a way of defining the current active list ... 📝

@nelsonic
Copy link
Member Author

nelsonic commented Aug 8, 2023

Default lists:
image

really wish pgweb had navigation; the http://localhost:8081/# URL never changes no matter what you're viewing. 🙄
Feature Request Reported: sosedoff/pgweb#625 (comment) 🤞

image

@ndrean hopefully this answers your question dwyl/learn-postgresql#94 (comment) 😉
But if you've found another Open Source + Fast startup time + Easy to extend (i.e. not React.js) let us know! 🙏

nelsonic added a commit to dwyl/book that referenced this issue Aug 9, 2023
@ndrean
Copy link

ndrean commented Aug 9, 2023

@nelsonic You can run livebook server ? and run Postgrex queries ? (if Postgres server is running and open on 5432)

Mix.install([
  {:kino_db, "~> 0.2.1"},
  {:postgrex, "~> 0.16.3"}
])
opts = [
  hostname: "localhost",
  port: 5432,
  username: "postgres",
  password: System.fetch_env!("LB_PASSWORD"),
  database: "my_db_dev"
]

{:ok, conn} = Kino.start_child({Postgrex, options})
Postgrex.query!(conn, "select * from users;", [])

@nelsonic
Copy link
Member Author

Sadly/frustratingly, I've been wrestling with Postgres for the past day trying to write a fairly basic query:

SELECT id
FROM list_items
WHERE person_id=2
GROUP BY list_id, item_id
image

ERROR: pq: column "list_items.id" must appear in the GROUP BY clause or be used in an aggregate function

This is the simplest GROUP BY query I could think of:

SELECT id, list_id, item_id
FROM list_items
GROUP BY (list_id, item_id)
image

Still the same error. 😢

So I read: https://learnsql.com/blog/must-appear-in-group-by-clause/

image

And tried:

SELECT list_id, item_id, COUNT(*)
FROM list_items
GROUP BY list_id, item_id
ORDER BY count DESC

Which works:
image

But the moment I try to do anything more advanced e.g:

SELECT list_id, item_id, COUNT(*), position
FROM list_items
GROUP BY list_id, item_id
ORDER BY count DESC

So that I can view the position of in the list I'm back to the same error:

image

I feel like I'm fighting against a fundamental flaw in Postgres that doesn't allow me to write the query I want. 😢

@nelsonic
Copy link
Member Author

After reading: https://stackoverflow.com/questions/19601948/must-appear-in-the-group-by-clause-or-be-used-in-an-aggregate-function

Which gives the example:

SELECT DISTINCT ON (cname) 
    cname, wmname, avg
FROM 
    makerar 
ORDER BY 
    cname, avg DESC ;

I tried:

SELECT DISTINCT ON (list_id, item_id)
  list_id, item_id, position
FROM list_items

Which works:
image

But the moment I try to ORDER BY a column e.g: position I'm back to a similar error:
image

ERROR: pq: SELECT DISTINCT ON expressions must match initial ORDER BY expressions

This is a pretty fundamental limitation and doesn't make any sense to me. 😢

I can write a Map |> Reduce in Elixir to achieve the result I want but I really wanted to do it in Postgres!

Googled for: https://www.google.com/search?q=postgres+group+by+two+columns+sort+by+a+third
Read: https://stackoverflow.com/questions/70670433/postgresql-group-by-multiple-columns-and-sort-within-each-group 👀
And learned about the LAST_VALUE() Window Function
https://www.postgresqltutorial.com/postgresql-window-function/postgresql-last_value-function/

This almost does what I want:

SELECT DISTINCT item_id, list_id, person_id,
  LAST_VALUE(position) OVER(
    PARTITION BY position
    ORDER BY position ASC
  ) pos,
  position
FROM list_items
WHERE person_id=2
ORDER BY pos ASC

dwyl-mvp-duplicate_list_items

There are still duplicates in the results even though I asked for SELECT DISTINCT item_id, list_id, person_id, ...
It's the same result if we surround the DISTINCT line with brackets: DISTINCT (item_id, list_id, person_id):

SELECT DISTINCT (item_id, list_id, person_id),
  LAST_VALUE(position) OVER(
    PARTITION BY position
    ORDER BY position ASC
  ) pos,
  position,
  id
FROM list_items
WHERE person_id=2
ORDER BY id DESC

dwyl-mvp-duplicate_list_items-ORDER_BY-id-DESC

It's clear that the DISTINCT query is not doing anything here.
So I'm going to remove it.


Along the way I used https://www.eversql.com/sql-syntax-check-validator/ to validate the query I was writing:

image

@nelsonic
Copy link
Member Author

Ok. after going down an annoying rabbit hole in Postgres I've realised that I can't perform the full aggregation query in SQL because I still have to accumulate all the timers ... 🙄

def accumulate_item_timers(items_with_timers) do

So I'm just going to write the sorting algo in Elixir and add it to the pipeline. 🧑‍💻 💭

@nelsonic
Copy link
Member Author

Going to pick this up tomorrow morning when I have a fresh head.
Way too many distractions & noise around me right now.

@panoramix360
Copy link
Collaborator

hey @nelsonic, I saw the new DB update you are modeling.

Can you share what you want the Query to return? I can try to help, I have some experience with PostgreSQL.

Reading your messages I didn't get the full requirement for the query.

@nelsonic
Copy link
Member Author

For context, the items_with_timers/1 function and related SQL query currently looks like this:

mvp/lib/app/item.ex

Lines 205 to 211 in 4a04f86

def items_with_timers(person_id \\ 0) do
sql = """
SELECT i.id, i.text, i.status, i.person_id, t.start, t.stop, t.id as timer_id FROM items i
FULL JOIN timers as t ON t.item_id = i.id
WHERE i.person_id = $1 AND i.status IS NOT NULL
ORDER BY timer_id ASC;
"""

Running this query on a working version of the MVP on localhost that has a few timers on the items
Results in the following table:

image

i.e. there are rows that look like duplicates but are in fact representing the multiple distinct timers.
Then the accumulate_item_timers/1 function will consolidate the rows and sum the timers:
https://github.com/dwyl/mvp/blob/4a04f8671edf329fd24cc87458a5ab86a110f1e0/lib/app/item.ex#L333C7-L376

What we want, in order to start using lists, is to modify the items_with_timers/1 SQL query to include a JOIN through list_items. But since we don't yet have the Interface Definition (AKA "UI/UX") for how to manage multiple lists,
We're just fetching "All items" and displaying them in a single view.

I've got this figured out. I just don't get large blocks of time get my work done each day. ⏳ 😞

Getting into this now. 🧑‍💻

@nelsonic
Copy link
Member Author

This is the SQL query I'm testing with:

 SELECT i.id, i.text, i.status, i.person_id, 
   t.start, t.stop, t.id as timer_id, 
   li.position as position, li.list_id
 FROM items i 
 FULL JOIN timers AS t ON t.item_id = i.id 
 JOIN list_items AS li ON li.item_id = i.id 
 WHERE i.person_id = 2 
 AND i.status IS NOT NULL 
 AND li.position != 999999.999
 ORDER BY li.position ASC;

image

Busy writing tests, docs and code to use this in lists and reordering #145

@nelsonic
Copy link
Member Author

After the changes I've made I have 6 failing tests. 💔
Busy fixing them now to ensure that my update to the items_with_timers/1 query
has not "broken" any existing functionality. 🧑‍💻 🤞

@nelsonic
Copy link
Member Author

For complete clarity: the tests that are failing relate to Drag-and-Drop events added in #345 🐉
Not any of the core functionality. 👌
Don't worry. 😉

nelsonic added a commit that referenced this issue Aug 11, 2023
nelsonic added a commit to dwyl/book that referenced this issue Aug 11, 2023
@nelsonic nelsonic mentioned this issue Aug 12, 2023
7 tasks
@nelsonic nelsonic unpinned this issue Aug 12, 2023
@nelsonic nelsonic changed the title EPIC: lists EPIC: lists Schema + Setup Aug 12, 2023
nelsonic added a commit to dwyl/book that referenced this issue Aug 12, 2023
nelsonic added a commit to dwyl/book that referenced this issue Aug 12, 2023
nelsonic added a commit that referenced this issue Aug 14, 2023
nelsonic added a commit that referenced this issue Aug 14, 2023
@nelsonic
Copy link
Member Author

Minor update: list.text to list.name which IMO is clearer:
dwyl-mvp-lists-erd

@nelsonic
Copy link
Member Author

nelsonic commented Aug 17, 2023

The list_items schema/table now has just fields:
dwyl-mvp-ERD-with-list_items

  1. list_id - the id of the list
  2. person_id - the id of the person who updated the list_items record
  3. seq - the sequence of item_ids e.g: "1, 2, 3, 42, 31, 26, 59" etc.

We use this seq will be used to find the items using a WHERE i.id IN ("1, 2, 3, 42, 31, 26, 59") query. I'm banking on the fact that this query will maintain the order. But if it doesn't we can easily just return the list.seq and perform the sorting on the client. 👌

Ref: https://www.reddit.com/r/PostgreSQL/comments/mlen8d/can_i_have_array_of_ids_in_where_statement/

nelsonic added a commit that referenced this issue Aug 23, 2023
nelsonic added a commit that referenced this issue Aug 30, 2023
nelsonic added a commit that referenced this issue Aug 31, 2023
@nelsonic
Copy link
Member Author

ERD with item.cid before lists:

image

With lists:

mvp-erd-with-list

With list_items:

mvp-erd-with-list_items

@nelsonic
Copy link
Member Author

This is included in #345

@nelsonic
Copy link
Member Author

nelsonic commented Sep 5, 2023

Removed list_items schema in #413 ✂️
So the ERD is now simplified to:
mvp-erd-lists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Share your constructive thoughts on how to make progress with this issue enhancement New feature or enhancement of existing functionality epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. priority-1 Highest priority issue. This is costing us money every minute that passes. technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: Done
Development

No branches or pull requests

3 participants