Add migration with licenses and license_limits tables #373
Conversation
| FROM license_limits | ||
| WHERE tenant_id = p_tenant_id | ||
| AND started_at <= p_at_time | ||
| ORDER BY started_at DESC |
There was a problem hiding this comment.
Yeap, we need to get the last limit, but not the future one. This will be the current limit.
| END IF; | ||
|
|
||
| ELSIF TG_OP = 'UPDATE' THEN | ||
| IF OLD.created_at != NEW.created_at THEN |
There was a problem hiding this comment.
can't we just add both, new and old value, for simplicity, as we have DISTINCT in SELECT DISTINCT unnest(v_check_times) AS check_time?
Probably IS NOT NULL can be added to it:
SELECT DISTINCT unnest(v_check_times) AS check_time WHERE unnest IS NOT NULL
| ELSIF TG_OP = 'DELETE' THEN | ||
| v_check_times := v_check_times || OLD.created_at; | ||
| IF OLD.expires_at IS NOT NULL THEN | ||
| v_check_times := v_check_times || OLD.expires_at; | ||
| END IF; | ||
| END IF; |
There was a problem hiding this comment.
do we actually need to check limits on license delete?
There was a problem hiding this comment.
I don't think so, fixed.
There was a problem hiding this comment.
I guess it could be a block with return to skip check even for NOW()
ELSIF TG_OP = 'DELETE' THEN
RETURN OLD;
END IF;
| v_check_times := v_check_times || OLD.created_at || NEW.created_at; | ||
| ELSE | ||
| v_check_times := v_check_times || NEW.created_at; | ||
| END IF; | ||
|
|
||
| IF OLD.expires_at IS DISTINCT FROM NEW.expires_at THEN | ||
| IF OLD.expires_at IS NOT NULL THEN | ||
| v_check_times := v_check_times || OLD.expires_at; |
There was a problem hiding this comment.
It's also possible that there is no need to check OLD.created_at and OLD.expires_at on license update
Because update is actually same as DELETE and then INSERT
But this is only if check upon deletion is really not necessary
There was a problem hiding this comment.
No, it makes sense if we're clearing expires_at for a license that expires next month. Without adding OLD.expires_at to v_check_times, we'll get exceeded limit next month.
No description provided.