Discourse likes, now with extra likes!

(Matches) #1

We at TDWTF wanted to share our love, so we’ve mastered the art of the DOUBLE like.

We wanted to bring this to you, because we like you.

Initial report:

Repro steps:

(Matches) #2

(Sam Saffron) #3

I think I know what it is, we have a unique index but postgres does a pretty poor job dealing with null in an index.

for example

create unique index idx on table(a,b)

insert into table(a,b) values (1, null)
insert into table(a,b) values (1, null) -- still valid

select count(*) from table
> 2

(Jeff Atwood) #4

Looks legit, I see dupe likes by matches on that post here.

(Sam Saffron) #5

We have:

add_index "post_actions", ["user_id", "post_action_type_id", "post_id", "deleted_at", "targets_topic"], name: "idx_unique_actions", unique: true

We should have

add_index "post_actions", ["user_id", "post_action_type_id", "post_id", "targets_topic"], name: "idx_unique_actions", unique: true, where: 'deleted_at IS NULL'

And targets_topic should not be nullable.


(Sam Saffron) #6

This should fix it, but I am nervous its late on a Friday and it needs proper testing

class FixIndexOnPostActions < ActiveRecord::Migration
  def change
    execute 'UPDATE post_actions SET targets_topic = false WHERE targets_topic IS NULL'
    change_column :post_actions, :targets_topic, :boolean, default: false, null: false

    execute '
    DELETE FROM post_actions pa
                    USING post_actions x
    WHERE pa.user_id = x.user_id AND
          pa.post_action_type_id = x.post_action_type_id AND
          pa.post_id = x.post_id AND
          pa.targets_topic = x.targets_topic AND
          pa.id < x.id AND
          pa.deleted_at IS NULL AND
          x.deleted_at IS NULL

    remove_index "post_actions", name: "idx_unique_actions"
    add_index "post_actions",
                ["user_id", "post_action_type_id",
                 "post_id", "targets_topic"],
                name: "idx_unique_actions",
                unique: true,
                where: 'deleted_at IS NULL'

@neil can you take this over and get it committed, just need you to validate that this delete statement is not haywire.

(Michael - DiscourseHosting.com) #7

No it doesn’t. It does a perfectly fine job. It follows the SQL92 standard. That specifies:

A unique constraint is satisfied if and only if no two rows in a table have the same non-null values in the unique columns.

Only MS-SQL treats two NULL values as equal in unique indexes.

(Konrad Borowski) #8

In SQL world, NULL doesn’t mean value that doesn’t exist, but rather, a value that is unknown. From this point of view, the behavior of UNIQUE constraint is actually logical. Two nulls could represent different values, therefore UNIQUE doesn’t affect them.


Question about the effect of this bug:

(Neil Lalonde) #10

The fix works in my testing, and no haywiring happened. :+1: I committed it.

(Jeff Atwood) #11