osmo-hlr[master]: Make subscr parameter to db_subscr_get() optional

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Tue Feb 21 11:42:06 UTC 2017


Patch Set 5:

> I think it's a very bad idea to intermix "check" and "update" logic
 > when it comes to DB.

You want an indicator to ensure that the IMSI exists. You can get it from the UPDATE operation. It is usually desirable to minimize database hits above all, so when we can halve the database hits needed to set a parameter, that's what we should do. Also I don't really like the "get" API with NULL param, and the code selecting all columns from a table and then not using them at all. So if we want to have a "does it exist" function I'd prefer it to have dedicated SQL in a separate function. ... but at the moment we don't need either, in my opinion.

Could we get other reviews to tip the scale?

-- 
To view, visit https://gerrit.osmocom.org/1853
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83b0f4a5dacb97614721690ef55bc1311624a58e
Gerrit-PatchSet: 5
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: No


More information about the gerrit-log mailing list