37

I am working with a coworker on a project that uses Inductive Automation software. If you don't know what it is, all you need to know is it provides a drag-and-drop GUI designer (based in java swing) and lets you write jython 2.5 or jython 2.7 (depending on the version) at different extension points of components. Press a submit button, run this jython script, like that. It's great for quickly getting something up and running (and for it's main purpose of interfacing wiht PLC's but that's not relevant here). But as a result, it allows you to shoot yourself in the foot if you aren't paying attention. As a side result to this often leads to very procedural code, no OOP almost ever. I only bring that up in the case that OOP might be an answer to the following issue I am facing.

We recently had a problem of duplicate records in a database. My coworker said this was caused when people would double or triple click the button, running the jython script multiple times. My suggestion was to make a UNIQUE index on whatever it is that defines the uniqueness of the table, so that if someone presses the button 3 times, we get that first record, but the next two are discarded as they violate the constraint. This would also allow us to do a try/except, catch the error thrown back by the violated constraint and do something with that information if we wanted, like tell the user to slow down.

My coworker said my solution was just masking the problem, that we should fix the script so it doesn't do the duplicate inserts. This would require making it so the button could only be pushed once and then is disabled from future button presses until the script completes, or sometimes having a statement that checks the database for existence of the record first before inserting. I've explained the issue with the second way, that if someone double pressed the button super quickly, you could have two scripts running at the same time, checking the database table, seeing no duplicate record, and then running two inserts. But he insists then that we should script out that error.

I'm still relatively new to the software field, just getting into my third year while my coworker is the most senior person at the company, so we are going to be doing things his way. However, I can't shake the feeling that we are going about this wrong.

Whenever I make some personal application I always use a UNIQUE constraint when appropriate to avoid duplicates, but now I am wondering if that is a mistake. Can someone more experienced share their view? Is there a right way or are there good use cases for both ways?

Edit: Wow this blew up. So the main issue was that the coding/scripting part had a lot of race conditions that would have taken a while to refactor with a deadline pressing, and the issue where a person could double or triple click a button before the window was changed. It's supposed to be someone clicks the button, some logic is run and the window changes. But while the logic is running the button is still clickable, can be pressed a few times, hence duplicates.

To eliminate it at a scripting level would require eliminating race conditions and how all our submit buttons work/windows are opened and putting some logic into background threads etc - which SHOULD be done no doubt, but also I feel like we SHOULD have a UNIQUE INDEX as well to prevent these double-clicks/race conditions from creating unintended duplicates.

A lot of good info here. I appreciate all the input.

13
  • 70
    It is possible that both of you are right. :) Enforcing that your data model is correct through constraints is indeed generally a good practice, but that does not mean you shouldn't design your application to handle the use case correctly as well. Nov 30, 2021 at 14:27
  • 3
    I think the title may be a little clearer if reworded. Dec 1, 2021 at 4:22
  • 2
    @BrianKarabinchak If you handle only single-threaded, no async code then yes it's easy to avoid duplicating data. However once you can have more than one thing access the DB concurrently it becomes hard to avoid. I mean, you can use locks... but, WTF, the DB already has locks built-in to avoid these problems! Let it do its job! Also, in Python the philosophy is more along the lines of "ask for forgiveness" than asking for permissions e.g. you don't see if a file exists before opening it, you just open it and if that fails you handle it... same here, insert and catch the exception if thrown
    – GACy20
    Dec 1, 2021 at 8:54
  • 1
    BTW, depending on how your database is designed, a UNIQUE index might NOT be the solution to your (specific) problem. Not that you shouldn't do it; you should've do it from the start, but that's another story
    – Josh Part
    Dec 1, 2021 at 16:48
  • 2
    Just a clarification: Apparently, the problem exists when a button is clicked twice in quick succession, but it is fine to click the button twice with sufficient time between clicks? Dec 1, 2021 at 18:42

13 Answers 13

133

My suggestion was to make a UNIQUE index on whatever it is that defines the uniqueness of the table

You really ought to have one already.
Uniqueness of rows is a pretty fundamental property of a table.

My coworker said my solution was just masking the problem, that we should fix the script so it doesn't do the duplicate inserts.

You're both right.
Changing the code [one place in] will help.
A constraint in the database will help more and it will protect you against any other places in the code that do the same thing.

7
  • 16
    and of course the index will make searching much faster :)
    – jwenting
    Dec 1, 2021 at 4:20
  • 3
    After you introduced a unique index you also have a wonderfully simple way your procedure can check for, well, uniqueness ;-). Dec 1, 2021 at 6:23
  • 25
    It won’t just protect in other places in code. It will also protect against other users submitting the same data. No button locking is gonna prevent that. Dec 1, 2021 at 9:39
  • 8
    I think a really important point to bear in mind is that your application may not always be the only front end via which your DB is accessed. Having the database properly constrained is always a good idea.
    – Paddy
    Dec 1, 2021 at 11:14
  • 3
    If you have multiple clients - or even threads updating the database means that you need to have a check in the database for uniqueness as you cannot catch all cases across different updates.
    – mmmmmm
    Dec 1, 2021 at 16:04
69

UNIQUE constraints are supported by all relational databases for good reasons, one of them being able to prevent data inconsistencies.

Your colleague has a point in that the code should be written to ensure that no duplicate records are inserted, but in practice code sometimes does things it's not intended to do, because it's doing what was written, not what was intended, and humans make mistakes. It's especially easy to make mistakes on concurrent code because there can be border cases that you did not think about.

Look at an analogy: Using seatbelts or airbags isn't a mask for sloppy driving, but can prevent or reduce serious injuries in case of accidents. The same is true for mandatory array bounds checks, automatic memory management, or database constraints. With perfect coding you would not need them, but that's just wishful thinking, error-free code is a myth.

6
  • 2
    You made my day with your seat-belt analogy :-)
    – Christophe
    Nov 30, 2021 at 17:57
  • 1
    The seatbelt analogy is great, I will be using that. Thanks! Nov 30, 2021 at 19:26
  • 7
    If you look carefully, you'll find many places like that in your programming tools. Strict typing; template types; public/private/protected; const; requirement to declare and initialize variables before use - just to name a few. You can do everything without these things (see PHP), but they really help you to avoid accidents.
    – Vilx-
    Nov 30, 2021 at 22:55
  • 2
    And, to take the analogy in another direction, that you have a car with airbags does not mean that you shouldn't also wear a seat belt. Dec 1, 2021 at 13:10
  • @JohnBollinger I remember being hit by an airbag as being a very unpleasant experience (even though they speed was relatively low; I was in the passenger seat, btw). Stopping at the seatbelt alone feels much preferable as long as it's sufficient. So I guess the analogy holds well here, as it's much preferable to be stopped by the UI instead of being hit in the face by a database error.
    – Frax
    Dec 3, 2021 at 1:29
20

Integrity of the data is, or should be, much more important than end user happiness. Put in the UNIQUE constraint ASAP. Don't let your coworker argue you out of it. If duplicate rows in your table are a problem, make them impossible no matter how users behave with their buttons.

Once users are seeing errors because they click too quickly, instead of generating bad data when they click too quickly, then you can consider UX changes that make those errors impossible or less likely.

1
  • 16
    "Integrity of the data is, or should be, much more important than end user happiness" - I'd say integrity of the data is extremely important to user happiness, much more than some UX improvements. Dec 1, 2021 at 10:24
14

TLDR; Absolutely fix this at the ui layer and add absolutely add a unique index if it fits the data model.

I am sorry I think some of the answers are missing a big point here. A unique index is an amazing tool that should be used as much as possible when it fits the data model. Data integrity and not relying on the "client" in the data model is key for building resilient apis and unique indexes can absolutely help with that. But using a unique index to fix a race condition should only be done when it fits the data model.

We don't have enough context here to say a unique index is the best solution since it may end up breaking valid user input. "whatever it is that defines the uniqueness of the table" might not work because there might not be a valid business case for unique enforcement. My stock order shouldn't fail the second time if I buy 20 shares at 102.12 dollars and then make the same order 5 minutes later because a unique index for (user, ticker, amount, price) was added to fix a race condition.

8
  • Good point. There are possible mechanisms to handle this (such as creating a transaction number which is then used as the unique row attribute) which may also be applicable to OP's situation. Dec 1, 2021 at 9:29
  • A stock order at one point in time is a distinct entity from another at a different point in time. The index should reflect that.
    – Andy Dent
    Dec 1, 2021 at 11:24
  • 1
    +1, well put. The way the question was phrased kinda steered us into "the data is bad", but you're right, the model might be weird enough that things that appear to require unique from our distance, actually can't be unique in the model. Dec 1, 2021 at 18:14
  • 1
    @pllee a transaction id could be requested from the server prior to opening the form, and be sent as one form field on submit. The technique can also be used in a slightly extended form for optimistic locking when editing existing records (the client would post old and new transaction ids on submit, and the server only accepts the update if the record in database currently has the old tx id). Dec 2, 2021 at 5:26
  • 1
    @BrianKarabinchak Then yeah a unique index would be great to add. I would also fix it on the frontend as well :) Fixing it on the front-end would give a better ux and show less errors once the unique index goes in.
    – pllee
    Dec 9, 2021 at 15:44
9

You are both absolutely right.

Software should be robust. That means it should have multiple safeguards against failure. Your database is the first point to start. Uniqueness of records is a pretty fundamental property, and literally every RDBMS supports UNIQUE constraints.

Having a unique constraints in your database will be your "last line of defense", but also a pretty robust one. It ensures that your Jython scripts -- which will have errors, as you just discovered -- cannot create duplicate records. And it does so globally, not only for one, but for all scripts that currently exist and will exists in the future. It also ensures no-one who accesses the database in other ways -- e.g. other programs feeding data into it -- can violate that constraint.

But fixing your code is also important. Just catching the database error is the quick and dirty solution and not great UX, its a terrible experience for the user if they can create error messages by clicking fast enough, and even more terrible if you silently drop the error, because that way the user doesn't even know when other errors -- which they need to know about -- come up. So fixing the user interface and Jython scripts -- disabling the button so that it can only be clicked once -- is also very important.

The best way to go about it would be not to fix this for this one Jython script, but to come up with a generic solution (a button that disables itself and enables again only after it has gotten the ok from the database) that can be used not only in this place, but everywhere you need this behavior.

Otherwise, you fix it in only one place, and in other places this can still happen. But if you use a unique constraint in your database, at least you'll now about it and an error will occur. This means you can then fix your UI scripts.

So do both, its the right way to handle that. The unique constraint robustly stops any constraint violation, the fix for the UI makes sure everything is handled gracefully.

The database is a tool. You can use unique constraint to make ure your data is the way you want it. There is no reason whatsoever not to use it, and it will help surface errors not only in one script, but also in others, so that you can address them.

2
  • 2
    Note that disabling the button is not water proof, while using indexes is. We had users in our own app who had poor internet connectivity, so the initial request failed. So the users tried it again and then we had 2 entries created. We had a 2 general problem until we ignored repeated requests
    – Ferrybig
    Dec 1, 2021 at 18:11
  • @Ferrybig Yes, absolutely! That is why you use the UNIQUE index, because thats actually what gives you the guarantees you need. The UI stuff is merely for convenience.
    – Polygnome
    Dec 2, 2021 at 9:22
4

No, the database constraint does not mask a bad script, instead the database constrain will help you discover bad scripts. As it currently stands, you have the worst of all possible worlds, bad data being created invisibly.

Adding a db constraint will result in bad data being rejected and failed scripts. At that point you have the option of changing the scripts to a) invisibly hide such errors or b) not attempt to create bad data. B is obviously the better approach, but you can only get there by adding the database constraint, without the constraint, you are always stuck in a world where bad data can be created at any time without notice.

2

Databases are often accessed by multiple applications. For instance there may be customer application, a CSR application, an order fulfillment application and a management reporting application. If there are no constraints on the database, then a race condition in a client application can insert duplicate records that break other applications. I would always add constraints to the database to prevent this. It simply is an easy fix that will prevent many production bugs and will show up application problems during development and testing.

1

Unlike others I might even say that you should not add the UNIQUE index.

We recently had a problem of duplicate records in a database. My coworker said this was caused when people would double or triple click the button, running the jython script multiple times. My suggestion was to make a UNIQUE index on whatever it is that defines the uniqueness of the table, so that if someone presses the button 3 times, we get that first record, but the next two are discarded as they violate the constraint.

This sounds like sometimes users do an action twice in error. Because of GUI. But it doesn't imply that such actions are always errors it should never be done twice. Just that it's a common error that happens because of how GUI is and it was discovered by noticing too many neighbouring "duplicates" in the database.

UNIQUE and whatever other constraints should be imposed when data itself implies that certain field/fields must be unique. In a case of a visual programming tool there's probably no such objective uniqueness case at all.

1
  • This is a very good point. That fact that the duplicated entries they encountered are erroneous doesn't necessarily imply that this is always an error. However, it sounds from the descirption like there actually are some fields that are expected to be unique and where UNIQUE constraint would be fitting, so it's really hard to say for sure whether in OPs case this is applicable.
    – Frax
    Dec 3, 2021 at 1:39
0

What you need is indeed a unique constraint, and maybe one that generates the keys automatically on insert. Usually in databases this is called a primary key with for example auto increment in mysql or similar features in other languages.

The problem of double/triple clicking is a whole different thing and could be caught at database level if you have the user information. you could for example add a constraint that you can only insert if there is no record (from your user) with time difference +/- 1 second or whatever makes sense in our app.

Generelly I believe such things should be caught inside a database transaction, that will get rolled back to a clean state if it fails. Trying to catch this outside in the business layer or even in th UI will lead to lots of headaches. You can try there additionally, but the atomic check in the database should be the final line of defense.

Just think about scaling your service up and having multiple instances of your backend running. A triple click could end up on three different processes/machines, making it hard to synchronize. If you do that in the database it is easy. First one wins, the other two violate the constraint and get rolled back.

0

It is not possible to guarantee perfect coordination in a distributed system. This has been mathematically proven: Two Generals Problem. (It was actually the first computer communication problem proven to be unsolvable.)

That means that as soon as you have two separate connections (to anything) there is a possibility of a race condition leading to two conflicting requests. (Like if you ever have two users sharing one database, or even one user running two copies of your application connected to the same database.) This is simply unavoidable.

A traditional RDBMS, however, is not distributed and can guarantee that it is ACID (atomic, consistent, isolated, durable)...the details of how this happens are very interesting, but the point you care about here is that your database is guaranteeing to be consistent -- even if two conflicting requests arrive simultaneously, the end state of your database will respect all of the rules that you have implemented (triggers, constraints, etc.). The result of this is that your mildly unfortunate user sometimes gets an error saying that their change couldn't go through, instead of being very unfortunate and getting the application into an inconsistent state where nothing works at all.

So you need constraints if you will ever possibly have more than exactly one thing happening at a time. Also, constraints are so easy and prevent developers from accidentally messing up in other ways, there are very few reasons not to use them when you can, whenever some natural constraint exists.

(If your application has scaled to the point where you need a distributed database or an overhead measured in single digit milliseconds is unacceptable then you have to settle for eventual consistency, but by then you have a lot of very, very smart people working very long hours together to figure this stuff out. And there is still an error saying that some transaction couldn't be completed, it just happens later on...like when you get an e-mail saying that actually the item you ordered was already sold out after all, even though it showed as being in stock.)

(And, yes, OK, this doesn't preclude having some nice check in the UI to show the user a nice error message. But you implement a lock on the door first in priority before you implement a nice sign to tell people the shop is closed. Ask your coworker what bad thing might happen if you have a constraint. IME the people who say "no constraints" when a constraint is appropriate are either very confused about the situations when a constraint is/isn't appropriate; or they are quite simply scared of the database, they don't know that much about it and would just prefer to avoid any kind of interaction/maintenance on it by pretending that it doesn't actually exist and is just some abstract entity represented in their ORM. If your co-developer is in the first category there is some hope of education. The second category can be rather harder, developers who don't want to learn new things are terrifying.)

0

The DB is responsible for storing your data in a consistent way.

The GUI is responsible for presenting your data to the user in a useful way.

The two aspects are related of course, but also still need to be separately looked at. It is not an "either/or", but a "both".

In this case: the UNIQUE needs to go into the database. Think about a situation where you need to work in the database with some DB tool that is not the GUI (i.e., through manual SQL statements, or through some batch job processing which runs in the night, automated). You want to have the data consistent no matter where the change comes from. As an extension, you also want to be robust against unforeseen changes or bugs in the GUI.

If you only do this, then in the future, when the GUI tries to create such duplicate entries again, there will be a hard error/exception. That is the issue that requires GUI changes as well. Obviously it would be best to change the GUI in a way that the problem cannot occur anymore. If that seems hard (or too much effort for the size of the issue), then maybe at least communicate the error in an understandable way - i.e., tell the user that the records have changed in the meantime and that he should just try again. In the worst case, the user gets an error message which is gibberish to him or her; but the data will still be consistent.

Maybe a final argument that may sway your co-worker (but this is not the primary reason for the decision): guaranteeing data consistency right in the DB is usually also much easier, both in the implementation (i.e., human time) as well as in execution (speed/resource usage).

0

Robustness, as well as security, is effective if implemented at every layer of your software architecture.

Like in the Swiss cheese model, software designers and implementors are responsible for implementing every possible check/constraint at every layer, from the UI to the database.

  • At the UI level

You are resposible for consistently disabling and re-enabling the button, or making sure that the button can't be clicked any more if used once.

Of course, if an error occurs, you are responsible for designing the proper logic (should I re-enable the button and let user try again?)

  • At the business level

The logic running the script should be resistant to multiple clicks. A simple and too straightforward way is to ignore requests coming too fast, called throttling

Multiple requests are related to the infamous POST warning that browsers trigger when you hit F5 on a POST request "You are submitting information twice, this could result in multiple purchases", which modern web applications mitigate multiple ways (e.g. using unique anti-replay tokens)

  • At the database level

And now, if a bad request made its way through the code to the boundary of the database, it's time to enforce a UNIQUE constraint as the last stand against bad data

Bottom line

DO implement unique constraints

DO implement additional validation on every layer of your software

DO NOT rely only on unique constraints

0

The UNIQUE index avoids two rows with the same key in your database. But is that all your script does, adding a row to the database? Could it do more things? Could it do more things in the future? You’ll have to examine very carefully if the UNIQUE index is enough to avoid trouble.

You definitely should also prevent the script being run twice, or prevent it from having an effect multiple times. For example, mark the record as “not added”, “in progress”, “added” and perhaps “rejected”. And the very first thing your script does is check that it is “not added” and change it to “in progress”.

You might then record the time when the record was added; if it was in progress for some hours you know something is dodgy. If the script runs within five seconds of the previous time you know it’s a second tap on the button.

Not the answer you're looking for? Browse other questions tagged or ask your own question.