6 ms·

I'd argue that the simple `deleted_at IS NULL` check is not broken - unless your product / domain specifically allows and requires scheduled future deletions ad

by madisp 4y ago

I'd argue that the simple `deleted_at IS NULL` check is not broken - unless your product / domain specifically allows and requires scheduled future deletions adding such logic can easily introduce bugs. For example, you could to get the comparison flipped by accident, and if it's only in one place out of many that bug could go unnoticed for a while.

doctor_eval 4y ago
You don't put the check in your code, you put it in the view, and access the data exclusively through that view. In that way, the check is defined exactly once.
- nightpool 4y ago
  And now your view is based on an unstable function, so it's impossible to write partial indexes for (very important for performance!) and impossible to use foreign keys with. Just add a check constraint that deleted at is never in the future and move on with your life. If you really, really need to schedule a delete, make it its own concern (UserVersions table with applicable_at?), don't mix it with your soft delete logic.
rodw 4y ago
If any non-null value for deleted_at indicates a logical delete, it seems like TIMESTAMP is the wrong data type for that column. I mean I guess I can imagine a scenario where you want to know _when_ a record was logically deleted in addition to a flag that says is_deleted, but maybe that should be two different columns. Or at the very least, `deleted_at` is a misleading name in that case. Interpreting `deleted_at` as "is deleted after but not before" is definitely a reasonable _enough_ interpretation. I agree it doesn't unambiguously mean that, but it's at best ambiguous enough to make for a bad column name if `IS NOT NULL` is the way you intend to check for a logical delete. > you could to get the comparison flipped by accident, and if it's only in one place out of many that bug could go unnoticed for a while. That's exactly why I proposed a database view as the proper solution to both whole `deleted_at IS NULL` "code leakage" issue. If you have a table that uses a logical-delete idiom that you intend to query from a bunch of different places in the code, a database view that filters out the logically deleted rows is a much more appropriate solution. It's a universally DRY solution. The single simple view covers all tech stacks, all services, etc., up to and including interacting with the database via some sort of REPL console. If you need to add `deleted_at IS NULL` to, say, 90% of the queries against the `customer` table and you're afraid someone is going to forget to do that somewhere somehow, it seems pretty obvious kind of "extract common WHERE clause" refactoring is warranted. This is especially true in the actual "business meaning" context here. You are only going to be interested in the logically deleted rows in small number of specific circumstances. Having some way to extract away that detail from the 90% of the use cases where "logical deleted" is meant to be functionally equivalent to "actually deleted" seems extremely warranted. It's a perfect counter-example to overcome an aversion to using database views. I get that the `active_customer` abstraction is probably awkward to implement in a lot of application code contexts (depending on your ORM/DB-query framework I guess), but the database view is a simple and elegant way to address that really clearly defined need.
- d0100 4y ago
  > If any non-null value for deleted_at indicates a logical delete, it seems like TIMESTAMP is the wrong data type for that column. Not if I also want the timestamp of when it was deleted
  - rodw 4y ago
    That's fair. It's what I was trying to acknowledge when I wrote "I can imagine a scenario where you want to know _when_ a record was logically deleted". That said, I'm genuinely curious about how often and in what way that time-based information is actually used. For example, I often see created_at and modified_at columns that are completely ignored by the actual business logic and application code that are only used (if ever) for ad hoc diagnostic purposes like reconstructing the sequence of events after some catastrophic error in the app logic screwed up the database. If run-away-logic-screwing-up-the-database is a legitimate cause for concern, that's probably reason enough to include that extra meta-data, but I can easily count on one hand the number of times that's come up in practice across my long career. I mean, I also include that sort of column often. And there are certainly entities for which those data are relevant for application logic and/or reporting purposes. But there are definitely cases in which those data are never used at all too. If this `deleted_at` timestamp-as-boolean convention is popular enough in the Rails community to be universally known, is there some common use for knowing _when_ a record was logically deleted (outside of reconstructing of the database state after something goes wrong) that I'm missing? I.e., is this something that's actually used in the typical business logic and application code, or is it primarily used for ad hoc debugging? To be clear I'm not _doubting_ that such a use case exists, it's just that off the top of my head I can't think of commonly occurring one.
- dubswithus 4y ago
  Every Rails developer knows what “deleted_at” means. I’m absolutely repulsed at the thought of turning the users table into a view. What’s next? No code and we only use triggers?
  - rodw 4y ago

by madisp 4y ago

doctor_eval 4y ago

You don't put the check in your code, you put it in the view, and access the data exclusively through that view. In that way, the check is defined exactly once.

nightpool 4y ago
And now your view is based on an unstable function, so it's impossible to write partial indexes for (very important for performance!) and impossible to use foreign keys with. Just add a check constraint that deleted at is never in the future and move on with your life. If you really, really need to schedule a delete, make it its own concern (UserVersions table with applicable_at?), don't mix it with your soft delete logic.

rodw 4y ago

If any non-null value for deleted_at indicates a logical delete, it seems like TIMESTAMP is the wrong data type for that column. I mean I guess I can imagine a scenario where you want to know _when_ a record was logically deleted in addition to a flag that says is_deleted, but maybe that should be two different columns. Or at the very least, `deleted_at` is a misleading name in that case. Interpreting `deleted_at` as "is deleted after but not before" is definitely a reasonable _enough_ interpretation. I agree it doesn't unambiguously mean that, but it's at best ambiguous enough to make for a bad column name if `IS NOT NULL` is the way you intend to check for a logical delete. > you could to get the comparison flipped by accident, and if it's only in one place out of many that bug could go unnoticed for a while. That's exactly why I proposed a database view as the proper solution to both whole `deleted_at IS NULL` "code leakage" issue. If you have a table that uses a logical-delete idiom that you intend to query from a bunch of different places in the code, a database view that filters out the logically deleted rows is a much more appropriate solution. It's a universally DRY solution. The single simple view covers all tech stacks, all services, etc., up to and including interacting with the database via some sort of REPL console. If you need to add `deleted_at IS NULL` to, say, 90% of the queries against the `customer` table and you're afraid someone is going to forget to do that somewhere somehow, it seems pretty obvious kind of "extract common WHERE clause" refactoring is warranted. This is especially true in the actual "business meaning" context here. You are only going to be interested in the logically deleted rows in small number of specific circumstances. Having some way to extract away that detail from the 90% of the use cases where "logical deleted" is meant to be functionally equivalent to "actually deleted" seems extremely warranted. It's a perfect counter-example to overcome an aversion to using database views. I get that the `active_customer` abstraction is probably awkward to implement in a lot of application code contexts (depending on your ORM/DB-query framework I guess), but the database view is a simple and elegant way to address that really clearly defined need.

d0100 4y ago
> If any non-null value for deleted_at indicates a logical delete, it seems like TIMESTAMP is the wrong data type for that column. Not if I also want the timestamp of when it was deleted
- rodw 4y ago
  That's fair. It's what I was trying to acknowledge when I wrote "I can imagine a scenario where you want to know _when_ a record was logically deleted". That said, I'm genuinely curious about how often and in what way that time-based information is actually used. For example, I often see created_at and modified_at columns that are completely ignored by the actual business logic and application code that are only used (if ever) for ad hoc diagnostic purposes like reconstructing the sequence of events after some catastrophic error in the app logic screwed up the database. If run-away-logic-screwing-up-the-database is a legitimate cause for concern, that's probably reason enough to include that extra meta-data, but I can easily count on one hand the number of times that's come up in practice across my long career. I mean, I also include that sort of column often. And there are certainly entities for which those data are relevant for application logic and/or reporting purposes. But there are definitely cases in which those data are never used at all too. If this `deleted_at` timestamp-as-boolean convention is popular enough in the Rails community to be universally known, is there some common use for knowing _when_ a record was logically deleted (outside of reconstructing of the database state after something goes wrong) that I'm missing? I.e., is this something that's actually used in the typical business logic and application code, or is it primarily used for ad hoc debugging? To be clear I'm not _doubting_ that such a use case exists, it's just that off the top of my head I can't think of commonly occurring one.
dubswithus 4y ago
Every Rails developer knows what “deleted_at” means. I’m absolutely repulsed at the thought of turning the users table into a view. What’s next? No code and we only use triggers?
- rodw 4y ago