DEV Community

Conner Phillis
Conner Phillis

Posted on

No Indexes, No Parameters, No Problem

This story is from an organization I worked at a while back. A team had recently lost all of its developers after they voluntarily left the company. Big red flag - but I was young and eager to prove myself, so I volunteered to take it on. The app had a multitude of issues: poor separation of concerns, copy-pasted code everywhere, awful performance, and outdated tech

I could probably write a whole series of posts on that application - it ended up costing me quite a bit of hair - but I wanted to talk about one of the craziest things I found.

The Bug

The first order of business was to fix the bugs that were preventing us from moving forward with customers that were using the software. There was a pretty standard create request that would fail somewhere inside of a 9,000 line function (yes... really...) for an unknown reason and customers were losing data. It had to be fixed.

The debugging tooling was terrible. The server was at the company's data center with no remote debugger access, so I had to build locally, drop DLLs on the machine loaded with logger statements, and read the output to figure out what went wrong. To make matters worse, the API always returned a 200 after swallowing the exception regardless of whether it succeeded or failed. Unfortunately, the fastest way to verify my changes was to query the database directly and check if the row inserted.

The primary key for the newly inserted record was something like 205, while the one previously was something like 190. This was weird - the entire time that I was debugging the application we hadn't even gotten to the point where we were inserting rows, there's no way we should have skipped that record.

The issue was fixed - so I had time to figure out why it was doing that. I hadn't previously gone line by line on the function, in favor of focusing on the area of the code that was failing. At the start of the function, was an ominous looking function call:

var id = DbUtils.GetNextId("TableName")
Enter fullscreen mode Exit fullscreen mode

dbo.Ids

Weird - I thought, why would we need a function to get the next id? SQL will create that for us, right?

The function definition contained a bit of SQL that looked something like this:

SELECT id FROM dbo.ids WHERE tableName = '<table-name>'
Enter fullscreen mode Exit fullscreen mode

followed by

UPDATE dbo.ids SET id = id + 1 WHERE tableName = '<table-name>`
Enter fullscreen mode Exit fullscreen mode

To date, I have yet to see anything in my career that made my jaw hit the floor that badly. I quickly selected against dbo.ids and found that every single table in the database had an entry in this singular table. Every single entity had to call DbUtils.GetNextId to get its next id.

Not only were we not using auto-incrementing primary keys, the operation to get the next id wasn't even atomic. If two calls came in at the same time and selected before one updated, two entities got the same id!!

Of course, I thought, at least the primary key constraint on the destination table should enforce uniqueness and keep us from getting into a bad state like that - that's just common sense, right?

Just in case, I opened SSMS & expanded the target table's indexes.

And it was empty.

I did a double-take, reloaded SSMS, and still empty. I checked my permissions to the database, nothing wrong - I was logged in as an SA. I checked another table - and empty there too.

Not one index in the entire database. Not a foreign key, a unique constraint, a primary key, nothing.

The dev prior to me hadn't known what an index was - and instead of doing research to figure out if there was a better way to do what he wanted, he just crapped out something that seemed like it worked.

Customers had always complained that bulk operations took forever - and now I knew why. Every single row insertion required at least three round trips to the database: two for the manual ID generation, and one for the actual insert. If only there was a better way...

The Aftermath

I ended up doing an audit of the entire codebase after that. It got worse. Not a single database parameter in the whole application. Every query was built with raw string concatenation. We were wide open to SQL injection.

There were enough bad findings in that application that I had to talk my boss in to putting an immediate moratorium on further sales. We had to rebuild the entire thing from scratch.

There were further issues to fix at customers we'd already deployed to, and the rewrite took at ton of work - but at the end of day cleaning up that hot pile of garbage taught me more than any class in my university could have. As many sleepless nights as it took - I would have to say that without that experience I wouldn't have learned nearly as much as I know today. In the end, it led me to a career I enjoy that keeps me happy and fed.


Anyways - I've always enjoyed reading software horror stories. Figured it was about time that I wrote my own worst software story. Thanks for reading - feel free to tell me about your own in the comments.

no AI was used to write this post, this is just how I type lol

Top comments (0)