Vinyl unique index by `is_nullable = true` field loses tuples #9769

d-enk · 2024-03-04T14:25:56Z

reproduced with 3.0.1-0-g31c2ddb, 2.11.2-0-g1bac2d257b

box.cfg({ log_level = "error" })

local space = box.schema.space.create("SPACE", {
    engine = "vinyl",
    format = {
        { name = "ID", type = "unsigned" },
        { name = "ANY", type = "scalar", is_nullable = true },
    },
    is_sync = false,
})

space:create_index("PRIMARY", {
    unique = true,
    type = "TREE",
    parts = { { field = "ID", type = "unsigned" } },
})

space:create_index("SECONDARY", {
    unique = false,
    type = "TREE",
    parts = { { field = "ANY", type = "scalar" } },
})

space:create_index("SECONDARY_UNIQUE", {
    unique = true,
    type = "TREE",
    parts = { { field = "ANY", type = "scalar" } },
})

local function show(...)
    print(require("json").encode({ ... }))
end

local function run(v1, v2)
    space:insert({ 1, v1 })

    box.atomic(function()
        space:replace({ 1, v2 })
        space:insert({ 2, v1 })
    end)

    print()
    show(space.index.PRIMARY:select({}, { fullscan = true }))
    show(space.index.SECONDARY:select({}, { fullscan = true }))
    show(space.index.SECONDARY_UNIQUE:select({}, { fullscan = true }))

    space:truncate()
end

local data = { 1, false, true, "" }

for _, vi in ipairs(data) do
    for _, vj in ipairs(data) do
        if vi ~= vj then
            run(vi, vj)
        end
    end
end

for _, v in ipairs(data) do
    run(v, box.NULL)
end

-- but

for _, v in ipairs(data) do
    run(box.NULL, v)
end

Out:

[[[1,false],[2,1]]]
[[[1,false],[2,1]]]
[[[1,false]]]

[[[1,true],[2,1]]]
[[[1,true],[2,1]]]
[[[1,true]]]

[[[1,""],[2,1]]]
[[[2,1],[1,""]]]
[[[1,""]]]

[[[1,1],[2,false]]]
[[[2,false],[1,1]]]
[[[1,1]]]

[[[1,true],[2,false]]]
[[[2,false],[1,true]]]
[[[1,true]]]

[[[1,""],[2,false]]]
[[[2,false],[1,""]]]
[[[1,""]]]

[[[1,1],[2,true]]]
[[[2,true],[1,1]]]
[[[1,1]]]

[[[1,false],[2,true]]]
[[[1,false],[2,true]]]
[[[1,false]]]

[[[1,""],[2,true]]]
[[[2,true],[1,""]]]
[[[1,""]]]

[[[1,1],[2,""]]]
[[[1,1],[2,""]]]
[[[1,1]]]

[[[1,false],[2,""]]]
[[[1,false],[2,""]]]
[[[1,false]]]

[[[1,true],[2,""]]]
[[[1,true],[2,""]]]
[[[1,true]]]

[[[1,null],[2,1]]]
[[[1,null],[2,1]]]
[[[1,null]]]

[[[1,null],[2,false]]]
[[[1,null],[2,false]]]
[[[1,null]]]

[[[1,null],[2,true]]]
[[[1,null],[2,true]]]
[[[1,null]]]

[[[1,null],[2,""]]]
[[[1,null],[2,""]]]
[[[1,null]]]

# but

[[[1,1],[2,null]]]
[[[2,null],[1,1]]]
[[[2,null],[1,1]]]

[[[1,false],[2,null]]]
[[[2,null],[1,false]]]
[[[2,null],[1,false]]]

[[[1,true],[2,null]]]
[[[2,null],[1,true]]]
[[[2,null],[1,true]]]

[[[1,""],[2,null]]]
[[[2,null],[1,""]]]
[[[2,null],[1,""]]]

Expected: index (3 line) doesn't lose tuple

Works correctly when is_nullable = false, engine = "memtx" or with other transaction combinations:

space:insert({ 1, v1 })
space:replace({ 1, v2 })
space:insert({ 2, v1 })

box.atomic(function()
    space:insert({ 1, v1 })
    space:replace({ 1, v2 })
end)

space:insert({ 2, v1 })

The text was updated successfully, but these errors were encountered:

8000

locker · 2024-05-16T10:13:17Z

A more compact reproducer:

local yaml = require('yaml')

box.cfg{log_level = 'error'}

local s = box.schema.space.create('space', {
    engine = 'vinyl',
    format = {
        { name = 'a', type = 'unsigned'},
        { name = 'b', type = 'unsigned', is_nullable = true},
    },
})
s:create_index('primary', {parts = {'a'}})
s:create_index('secondary', {parts = {'b'}, unique = true})

s:insert({1, 10})

box.begin()
s:replace({1, 20})
s:insert({2, 10})
box.commit()

print(yaml.encode({
    primary = s.index.primary:select({}, {fullscan = true}),
    secondary = s.index.secondary:select({}, {fullscan = true}),
}))

os.exit(0)

Output:

---
primary:
- [1, 20]
- [2, 10]
secondary:
- [1, 20]
...

Notes:

The problem disappears if any of the following changes is made:

is_nullable is unset for the second space field;
unique is unset for the secondary index;
box.begin and box.commit are removed.

A unique nullable key definition extended with primary key parts (cmp_def) assumes that two tuples are equal *without* comparing primary key fields if all secondary key fields are equal and not nulls, see tuple_compare_slowpath(). This is a hack required to ignore the uniqueness constraint for nulls in memtx. The memtx engine can't use the secondary key definition as is (key_def) for comparing tuples in the index tree, as it does for a non-nullable unique index, because this wouldn't allow insertion of any duplicates, including nulls. It couldn't use cmp_def without this hack, either, because then conflicting tuples with the same secondary key fields would always compare as not equal due to different primary key parts. For Vinyl, this hack isn't required because it explicitly skips the uniqueness check if any of the indexed fields are nulls, see vy_check_is_unique_secondary(). Furthermore, this hack is harmful because Vinyl relies on the fact that two tuples compare as equal by cmp_def if and only if *all* key fields (both secondary and primary) are equal. For example, this is used in the transaction manager, which overwrites statements equal by cmp_def, see vy_tx_set_entry(). Let's disable this hack by resetting unique_part_count in cmp_def. Closes tarantool#9769 NO_DOC=bug fix

A unique nullable key definition extended with primary key parts (cmp_def) assumes that two tuples are equal *without* comparing primary key fields if all secondary key fields are equal and not nulls, see tuple_compare_slowpath(). This is a hack required to ignore the uniqueness constraint for nulls in memtx. The memtx engine can't use the secondary key definition as is (key_def) for comparing tuples in the index tree, as it does for a non-nullable unique index, because this wouldn't allow insertion of any duplicates, including nulls. It couldn't use cmp_def without this hack, either, because then conflicting tuples with the same secondary key fields would always compare as not equal due to different primary key parts. For Vinyl, this hack isn't required because it explicitly skips the uniqueness check if any of the indexed fields are nulls, see vy_check_is_unique_secondary(). Furthermore, this hack is harmful because Vinyl relies on the fact that two tuples compare as equal by cmp_def if and only if *all* key fields (both secondary and primary) are equal. For example, this is used in the transaction manager, which overwrites statements equal by cmp_def, see vy_tx_set_entry(). Let's disable this hack by resetting unique_part_count in cmp_def. Closes #9769 NO_DOC=bug fix

A unique nullable key definition extended with primary key parts (cmp_def) assumes that two tuples are equal *without* comparing primary key fields if all secondary key fields are equal and not nulls, see tuple_compare_slowpath(). This is a hack required to ignore the uniqueness constraint for nulls in memtx. The memtx engine can't use the secondary key definition as is (key_def) for comparing tuples in the index tree, as it does for a non-nullable unique index, because this wouldn't allow insertion of any duplicates, including nulls. It couldn't use cmp_def without this hack, either, because then conflicting tuples with the same secondary key fields would always compare as not equal due to different primary key parts. For Vinyl, this hack isn't required because it explicitly skips the uniqueness check if any of the indexed fields are nulls, see vy_check_is_unique_secondary(). Furthermore, this hack is harmful because Vinyl relies on the fact that two tuples compare as equal by cmp_def if and only if *all* key fields (both secondary and primary) are equal. For example, this is used in the transaction manager, which overwrites statements equal by cmp_def, see vy_tx_set_entry(). Let's disable this hack by resetting unique_part_count in cmp_def. Closes #9769 NO_DOC=bug fix (cherry picked from commit 2e68906)

d-enk added the bug Something isn't working label Mar 4, 2024

Gumix added the vinyl label Mar 4, 2024

locker self-assigned this May 16, 2024

locker mentioned this issue May 16, 2024

vinyl: fix bug when tuple not committed to unique nullable index #10016

Merged

locker added 2.11 Target is 2.11 and all newer release/master branches 3.1 Target is 3.1 and all newer release/master branches labels May 17, 2024

locker closed this as completed in #10016 May 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vinyl unique index by `is_nullable = true` field loses tuples #9769

Vinyl unique index by `is_nullable = true` field loses tuples #9769

Vinyl unique index by is_nullable = true field loses tuples #9769

Vinyl unique index by is_nullable = true field loses tuples #9769

Comments

Vinyl unique index by `is_nullable = true` field loses tuples #9769

Vinyl unique index by `is_nullable = true` field loses tuples #9769