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.

fixing…


(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'
  end
end

@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.


#9

Question about the effect of this bug:
http://what.thedailywtf.com/t/the-official-likes-thread-topic-proof-that-jdgi/1000/13793


(Neil Lalonde) #10

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


(Jeff Atwood) #11