6 ms·

Seriously. That "Downsides: Code leakage" point is nonsensical. ``` CREATE OR REPLACE VIEW active_customer AS SELECT * FROM customer WHERE deleted_at

by rodw 4y ago

Seriously. That "Downsides: Code leakage" point is nonsensical. ``` CREATE OR REPLACE VIEW active_customer AS SELECT * FROM customer WHERE deleted_at IS NULL OR deleted_at <= NOW() ; ``` There, I fixed it. Just use `active_customer` instead of `customer ... deleted_at IS NULL`. In fact, since the deleted_at column is a timestamp, the original "leakage" query: ``` SELECT * FROM customer WHERE id = @id AND deleted_at IS NULL; ``` is actually broken. A non-null `deleted_at` timestamp that's in the future implies the record hasn't been deleted yet, right? I've often had junior devs assert that views are some kind of code smell, but these sorts of "canned query/filter that you want to apply very very often" seem like the perfect use case for a view to me. It's DRY, and the fact that your standard "query" is in the database" means you can change it more readily than trying to make sure you hit all the points it might be embedded in the application code. > I feel like a lot of developers underutilize the capabilities of the massively advanced database engines they code against Early-ish in the JDBC days a senior dev I was working with at the time (as a junior dev myself) made a pretty good case that "the database is part of the application" that's always stuck with me. Full database independence via software level abstractions is a pretty silly goal outside of library code. If you have a service that makes extensive use of the database, don't throw away the database features in the interest of some abstract "we could swap out oracle with mysql without changing anything" objective. If you want it to be generic, use the SQL standard, but don't be afraid to have a few db-specific bits in the app code if that's a subsystem you might replace once a decade or something. I blame the DBA/Dev divide for a lot of this. A lot of the impedance between these layers is social/procedural. If you can change the DB as easily as the code, there's a lot less fear of using the right tool for the specific job.

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.
djur 4y ago
The query isn't broken. In the Rails community at least it is very common to use a nullable frobbed_at column to indicate both "was it frobbed" and "when was it frobbed". In that context, the boolean check is always NULL/NOT NULL, rather than a time comparison.
- rodw 4y ago
  I can see that use case and I guess if that's a popular idiom within a particular dev community it's not indefensible, but from a first-principles code-complete/pragmatic-programmer kind of lens, that seems to me like a really terrible naming convention if that's your intent. It seems to me that having a date-type column named `is_frobbed` is highly preferable to `frobbed_at` if your intent is for `IS NOT NULL` to mean frobbed. Sure, it's a little weird to have boolean-sounding `is_frobbed` name for a date column, but the number of times that `frobbed_at IS [NOT] NULL` appears in the code is likely to dwarf the number of times you're inserting a date type into the `is_frobbed` column. I feel like there's always going to be someone (like me in this case) that's going to come across a `frobbed_at IS NOT NULL` case, notice that it's a date column and make the same flawed assumption I did. A similar thing will happen to the `is_frobbed` column too, but in that case trying to treat the date type as a boolean is going to make it obvious that you're not understanding the full context. Frankly, if I was naming a column for this idiom and didn't have other constraints (like the Rails context), I would probably try to find a more exact and direct way to express it, if that's a little clunky. Maybe something like `frobbed_when_not_null`? But I don't love that. Honestly it may make more sense to have a actual boolean-valued `is_frobbed` column and an independent `frobbed_at` timestamp column that's populated by a trigger when the value of `is_frobbed` changes. I feel like "clunky but direct" beats out "elegant but misleading" in the long run, especially given the degree of "clunky" and "elegant" we're talking about here. I don't expect to talk you out of it, and I'd fall in line with this in a Rails context too if that's the convention, but I think it's objectively poor design. For what it's worth, if the `frobbed_at` convention usually intended to track the timestamp at which the frobbing happened, the good news is that my `AND frobbed_at <= NOW()` check would be unlikely to break anything in practice. Assuming that there's nothing weird going on with the timestamps, the frobbed_at dates will always be in the past anyway.
doctor_eval 4y ago
> the database is part of the application 100% this. If you accept that the database is part of the application, you give yourself permission to use the full feature set of the database, and life becomes a lot simpler. Using views, stored procedures and other features lets you implement things like soft delete trivially, without it infecting all your application code. In my entire career I've changed backend databases for an application exactly twice. It's not easy, and no amount of abstraction is likely to make it easier.
- dubswithus 4y ago
  > Using views, stored procedures and other features lets you implement things like soft delete trivially, without it infecting all your application code. That’s great but some of us actually like to write code. Especially Ruby on Rails where soft delete is a breeze if you don’t overthink it and build something the business doesn’t need.
  - doctor_eval 4y ago
    Well, less code means less bugs, but go nuts. I'm just saying that the database is part of the stack. You wouldn't avoid Ruby features "just in case we stop using Ruby" - so why would we avoid using database features? It's up to you how best to assemble the features from your stack. There is a reason we avoided database features in the 90's, which was to avoid database vendor lock-in. This was almost entirely a financial decision - you didn't want to be at the mercy of a vendor who could 10x the license fees and put you out of business, so you needed to have leverage over the vendor, and you needed to be able to credibly say you could change databases -- and in fact this exact scenario actually happened to me. But that kind of behaviour is not really needed any more, and there's no need to avoid the great database features that systems like PG provide. In my experience, using database primitives to implement features tends to perform much faster (10x - 100x) and more consistently than the equivalent implementations higher up the stack.
    - dubswithus 4y ago
      The use case for using the database here is overblown. I've never experienced of heard of anyone using a view for the users table. I suppose it may exist in the wild somewhere. I wouldn't avoid Ruby features because the applications I develop are in Ruby and it would require a complete rewrite anyway. I avoid the database because there are development speed advantages to using Rails features. And everything works together nicely assuming you do things the Rails way. https://github.com/rubysherpas/paranoia https://github.com/rubysherpas/paranoia > If a record has has_many associations defined AND those associations have dependent: :destroy set on them, then they will also be soft-deleted if acts_as_paranoid is set, otherwise the normal destroy will be called. It's convenient.

by rodw 4y ago

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.

djur 4y ago

The query isn't broken. In the Rails community at least it is very common to use a nullable frobbed_at column to indicate both "was it frobbed" and "when was it frobbed". In that context, the boolean check is always NULL/NOT NULL, rather than a time comparison.

rodw 4y ago
I can see that use case and I guess if that's a popular idiom within a particular dev community it's not indefensible, but from a first-principles code-complete/pragmatic-programmer kind of lens, that seems to me like a really terrible naming convention if that's your intent. It seems to me that having a date-type column named `is_frobbed` is highly preferable to `frobbed_at` if your intent is for `IS NOT NULL` to mean frobbed. Sure, it's a little weird to have boolean-sounding `is_frobbed` name for a date column, but the number of times that `frobbed_at IS [NOT] NULL` appears in the code is likely to dwarf the number of times you're inserting a date type into the `is_frobbed` column. I feel like there's always going to be someone (like me in this case) that's going to come across a `frobbed_at IS NOT NULL` case, notice that it's a date column and make the same flawed assumption I did. A similar thing will happen to the `is_frobbed` column too, but in that case trying to treat the date type as a boolean is going to make it obvious that you're not understanding the full context. Frankly, if I was naming a column for this idiom and didn't have other constraints (like the Rails context), I would probably try to find a more exact and direct way to express it, if that's a little clunky. Maybe something like `frobbed_when_not_null`? But I don't love that. Honestly it may make more sense to have a actual boolean-valued `is_frobbed` column and an independent `frobbed_at` timestamp column that's populated by a trigger when the value of `is_frobbed` changes. I feel like "clunky but direct" beats out "elegant but misleading" in the long run, especially given the degree of "clunky" and "elegant" we're talking about here. I don't expect to talk you out of it, and I'd fall in line with this in a Rails context too if that's the convention, but I think it's objectively poor design. For what it's worth, if the `frobbed_at` convention usually intended to track the timestamp at which the frobbing happened, the good news is that my `AND frobbed_at <= NOW()` check would be unlikely to break anything in practice. Assuming that there's nothing weird going on with the timestamps, the frobbed_at dates will always be in the past anyway.

doctor_eval 4y ago

> the database is part of the application 100% this. If you accept that the database is part of the application, you give yourself permission to use the full feature set of the database, and life becomes a lot simpler. Using views, stored procedures and other features lets you implement things like soft delete trivially, without it infecting all your application code. In my entire career I've changed backend databases for an application exactly twice. It's not easy, and no amount of abstraction is likely to make it easier.

dubswithus 4y ago
> Using views, stored procedures and other features lets you implement things like soft delete trivially, without it infecting all your application code. That’s great but some of us actually like to write code. Especially Ruby on Rails where soft delete is a breeze if you don’t overthink it and build something the business doesn’t need.
- doctor_eval 4y ago
  Well, less code means less bugs, but go nuts. I'm just saying that the database is part of the stack. You wouldn't avoid Ruby features "just in case we stop using Ruby" - so why would we avoid using database features? It's up to you how best to assemble the features from your stack. There is a reason we avoided database features in the 90's, which was to avoid database vendor lock-in. This was almost entirely a financial decision - you didn't want to be at the mercy of a vendor who could 10x the license fees and put you out of business, so you needed to have leverage over the vendor, and you needed to be able to credibly say you could change databases -- and in fact this exact scenario actually happened to me. But that kind of behaviour is not really needed any more, and there's no need to avoid the great database features that systems like PG provide. In my experience, using database primitives to implement features tends to perform much faster (10x - 100x) and more consistently than the equivalent implementations higher up the stack.
  - dubswithus 4y ago
    The use case for using the database here is overblown. I've never experienced of heard of anyone using a view for the users table. I suppose it may exist in the wild somewhere. I wouldn't avoid Ruby features because the applications I develop are in Ruby and it would require a complete rewrite anyway. I avoid the database because there are development speed advantages to using Rails features. And everything works together nicely assuming you do things the Rails way. https://github.com/rubysherpas/paranoia https://github.com/rubysherpas/paranoia > If a record has has_many associations defined AND those associations have dependent: :destroy set on them, then they will also be soft-deleted if acts_as_paranoid is set, otherwise the normal destroy will be called. It's convenient.