[sql-27]: db/sqlc: kvstores schemas & queries #1028
Conversation
ellemouton
added
database
db/sqlc/queries/kvstores.sql
Outdated
| -- name: InsertKVStoreRecord :exec | ||
| INSERT INTO kvstores (perm, rule_id, session_id, feature_id, key, value) | ||
| VALUES ($1, $2, $3, $4, $5, $6); |
There was a problem hiding this comment.
I guess this would be more work on the interface side, but would it be worth it to instead have an upsert query for this and the update ones?
There was a problem hiding this comment.
can you explain a bit more? what is the benefit here?
There was a problem hiding this comment.
(happy to update, just want to understand the reason)
There was a problem hiding this comment.
(maybe also check the referencing PR to see how these queries are being used)
There was a problem hiding this comment.
the motivation was just to unify the Get and Update methods, to have less code, but it would also interact with the uniqueness constraint via ON CONFLICT
-- name: UpsertKVStoreRecord :exec
INSERT INTO kvstores (perm, rule_id, session_id, feature_id, key, value)
VALUES ($1, $2, $3, $4, $5, $6)
ON CONFLICT(key, rule_id, perm, session_id, feature_id)
DO UPDATE SET value = excluded.value;
or is it better to separate the write (update/delete) methods for the different kvstore types?
There was a problem hiding this comment.
oh i see! great idea 🙏
There was a problem hiding this comment.
this turned out not to work - the conflict doesnt seem to get caught properly for null values. reverting
There was a problem hiding this comment.
ok, thank you for trying!
ellemouton
changed the title
ellemouton
requested review from
bitromortac
and removed request for
ViktorTigerstrom and
bitromortac
ellemouton
force-pushed
the
sql27
branch
4 times, most recently
from
45bf999 to
1f3592d
Compare
| id INTEGER PRIMARY KEY, | ||
|
|
||
| -- The unique name of the rule. | ||
| name TEXT NOT NULL UNIQUE |
There was a problem hiding this comment.
This is creating an un-namned unique index, which may the same issues we've discussed previously in regards to updates of it :).
There was a problem hiding this comment.
cool - good idea. i originally thought that was mainly for composite indexes - but you're right, makes sense here too
In this commit, we define the schemas and queries that are needed to implement the firewalldb's KVStores in SQL.
In this commit, we define the schemas and queries that are needed to implement the firewalldb's KVStores in SQL.
NOTE: this PR does not make use of the new queries at all. See this draft PR for how these queries will be used.
See the schema here
This part of the #917 project. The PR after this one will use the new queries to implement a SQL version of the
RulesDBinterface.