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

Замечания по СРС 1 по СУБД #18

Open
ns-roxer opened this issue Nov 9, 2023 · 10 comments
Open

Замечания по СРС 1 по СУБД #18

ns-roxer opened this issue Nov 9, 2023 · 10 comments

Comments

@ns-roxer
Copy link

ns-roxer commented Nov 9, 2023

  1. https://github.com/go-park-mail-ru/2023_2_Umlaut/blob/db-dz-1/db/migrations/000001_init.up.sql - у некоторых таблиц явно не определен PK.
  2. Ничего не мешает добавить неограниченное количество одинаковых записей в user_tag, в этом есть смысл или это нужно исправить?
  3. https://github.com/go-park-mail-ru/2023_2_Umlaut/blob/db-dz-1/db/normalized/relations.md - раз сделали описание в mermaid, то стоило его добавить в сам .md файл, так как гитхаб умеет рендерить mermaid.js описания
  4. согласно DDL один юзер может наставить другому неограниченное количество лайков, это корректное поведение?
  5. Так же один юзер может инициировать неограниченное количество диалогов с другим (тем же самым) пользователем, это корректное поведение?
  6. liked_by_user_id и liked_to_user_id - слишком похожие названия, предвижу много путаницы при чтении поддержке запросов с участием этих полей. Переименуйте, пожалуйста, так, чтобы они не сливались, например, liked_from_user_id и liked_to_user_id - минимальное изменение, но теперь названия полей, хотя бы разной длины
@Max425
Copy link
Collaborator

Max425 commented Nov 9, 2023

Исправили, есть только вопрос
У нас в таблице dialog сейчас UNIQUE (user1_id, user2_id), но при таком индексе можно добавить пары (1, 2) и (2, 1), хотя между пользователями может быть только один диалог.

На данный момент мы решили эту проблему на стороне бэкенда и просто в user1_id передаем меньшее из пары, а в user2_id большее. Но можно ли создать такое ограничение, которое будет покрывать оба случая?

@ns-roxer
Copy link
Author

ns-roxer commented Nov 9, 2023

Замените

UNIQUE (user1_id, user2_id)

на

user_min_id INT GENERATED ALWAYS AS (LEAST(user1_id, user2_id)) STORED,
UNIQUE (user_min_id)

и будет вам счастье. Только обязательно разберитесь, что делает эта конструкция, в будущем я с вас это спрошу.

@ns-roxer
Copy link
Author

ns-roxer commented Nov 9, 2023

Насчет primary key в таблицах связях я рекомендую еще подумать, не во всех них есть смысл делать суррогатный ID вместе с составным уникальным индексом, ведь составной PK покроет обе потребности.

@ns-roxer
Copy link
Author

ns-roxer commented Nov 9, 2023

И еще такое замечание - не вижу нигде в таблицах created_at и updated_at полей. Требования такого явно не было, так что это не блокер, но я рекомендую подумать и добавить эти поля в те таблицы, в которых они по вашему мнению могут пригодиться.
Чтобы это понять, попробуйте оценить свой проект с точки зрения разработчика, который поддерживает его уже в продакшене с реальными данными.
С дефолтными значениями и триггером на update_at вам даже в коде приложения не понадобится эти поля учитывать, но при этом у вас будет очень полезная отладочная информация

@Max425
Copy link
Collaborator

Max425 commented Nov 10, 2023

user_min_id INT GENERATED ALWAYS AS (LEAST(user1_id, user2_id)) STORED,
UNIQUE (user_min_id)

В таком случае можно добавить пару (1, 2), но потом пару (1, 3) уже не получится вставить.

но можно вот так

user_min_id INT GENERATED ALWAYS AS (LEAST(user1_id, user2_id)) STORED,
user_max_id INT GENERATED ALWAYS AS (greatest(user1_id, user2_id)) STORED,
UNIQUE (user_min_id, user_max_id)

такой вариант работает, но добавление двух столбцов в таком случае нормальная практика?

@ns-roxer
Copy link
Author

мда, поторопился я с советом :)
Спасибо, что указали на ошибку.

На этом этапе будем считать, что первый рубеж пройдем и можно подумать над денормализацией, так что попробуйте поэкспериментировать с составными типами данных:
массив или hstore https://www.postgresql.org/docs/current/hstore.html

@ns-roxer
Copy link
Author

Вот что еще нашел, что вам стоит попробовать:

CREATE TABLE unique_pairs (
    id SERIAL PRIMARY KEY,
    first_value INTEGER NOT NULL,
    second_value INTEGER NOT NULL,
    CONSTRAINT unique_pair_constraint UNIQUE (LEAST(first_value, second_value), GREATEST(first_value, second_value))
);

@Max425
Copy link
Collaborator

Max425 commented Nov 10, 2023

Такой вариант мы тоже пробовали, но postgres ошибку синтаксиса возвращает, не дает так создать

@ns-roxer
Copy link
Author

мдэ, не PGшный синтаксис.

Тогда без добавления лишних полей можно так попробовать:

CREATE TABLE unique_pairs (
    id SERIAL PRIMARY KEY,
    first_value INTEGER NOT NULL,
    second_value INTEGER NOT NULL,
    CONSTRAINT check_pair_order CHECK (first_value < second_value),
    CONSTRAINT unique_pair_constraint UNIQUE (first_value, second_value)
);

Но на стороне приложения, вам нужно будет явно задавать порядок диалогов.
То есть со стороны базы строгое соблюдение ограничения целостности будет присутствовать и без лишних аттрибутов. Но на стороне приложения это потребует небольшой дополнительной обработки.

@Max425
Copy link
Collaborator

Max425 commented Nov 10, 2023

Спасибо, так и сделаем.
Тогда по СРС 1 кажется все поправил, теперь будем денормализировать, уже есть некоторые идеи)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants