Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Calling a Lua function from SQL changes the number of rows in the result! #5427

Closed
msiomkin opened this issue Oct 16, 2020 · 1 comment
Closed

Calling a Lua function from SQL changes the number of rows in the result! #5427

msiomkin opened this issue Oct 16, 2020 · 1 comment
Assignees
@ImeevMA
Labels
bug Something isn't working sql
Milestone
2.5.3

Comments

@msiomkin
Copy link
Contributor

msiomkin commented Oct 16, 2020 •
edited

Tarantool versions:
Tarantool Enterprise For Mac 2.3.1-154-g4616b51c4
Tarantool Enterprise 2.4.2-12-g11f587c
Tarantool 2.5.1-0-gc2d8c03ee

OS version:
Centos 8, Mac OS X

Bug description:
Calling a Lua function from SQL changes the number of rows in the result!

Steps to reproduce:

Run the following script (especially compare the first and the fourth queries):

#!/usr/bin/env tarantool

local log = require("log")

box.cfg{}

local chain = box.schema.space.create("chain", { if_not_exists = true })

chain:truncate()

chain:format({
    { name = "modelUid", type = "string" },
    { name = "hash", type = "string" },
    { name = "count", type = "unsigned" },
    { name = "percent", type = "number" },
    { name = "durationFactAvg", type = "integer" },
    { name = "durationSchAvg", type = "integer" },
    { name = "eventCount", type = "unsigned" }
})

chain:create_index("modelUid_hash", {
    unique = true,
    parts = { "modelUid", "hash" },
    if_not_exists = true
})

chain:create_index("modelUid_percent", {
    unique = false,
    parts = { "modelUid", "percent" },
    if_not_exists = true
})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    '0e4fbf755f62da00f64e8e658b041858b587d14c',
    1, 5, 520920, 520920, 5})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    '1599174857569a63bbbf2cb2168995381b82bdcd',
    5, 25, 259200, 259200, 4})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    '392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02',
    1, 5, 1576440, 1576440, 6})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    '767141a2b3fd130bcc5b419bf77b244359974943',
    1, 5, 1576440, 1576440, 7})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    '7b1abdfcb576c4877eb96ef28328313f3acb470e',
    3, 15, 703320, 703320, 5})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a',
    1, 5, 1368780, 1368780, 7})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    'bb9a5ad4eddbb17496957d5e63db463442c8e41f',
    4, 20, 1449180, 1449180, 5})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    'cd2e9d851b51e0ce0796db2f30ec315934a3cf9a',
    1, 5, 852300, 852300, 5})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f',
    2, 10, 1368780, 1368780, 7})

chain:insert({
    '85777b8f-35ac-40be-a733-647005ee4cfa',
    'f311956d23b2bb7193f60b87e195171b8178d666',
    1, 5, 259200, 259200, 4})


local chainEvent = box.schema.space.create("chainEvent", { if_not_exists = true })

chainEvent:truncate()

chainEvent:format({
    { name = "chainHash", type = "string" },
    { name = "name", type = "string" },
})

chainEvent:create_index("chainHash_name", {
    unique = true,
    parts = { "chainHash", "name" },
    if_not_exists = true
})

chainEvent:insert({'0e4fbf755f62da00f64e8e658b041858b587d14c', 'check ticket'})
chainEvent:insert({'0e4fbf755f62da00f64e8e658b041858b587d14c', 'decide'})
chainEvent:insert({'0e4fbf755f62da00f64e8e658b041858b587d14c', 'examine thoroughly'})
chainEvent:insert({'0e4fbf755f62da00f64e8e658b041858b587d14c', 'register request'})
chainEvent:insert({'0e4fbf755f62da00f64e8e658b041858b587d14c', 'reject request'})
chainEvent:insert({'1599174857569a63bbbf2cb2168995381b82bdcd', 'check ticket'})
chainEvent:insert({'1599174857569a63bbbf2cb2168995381b82bdcd', 'decide'})
chainEvent:insert({'1599174857569a63bbbf2cb2168995381b82bdcd', 'examine thoroughly'})
chainEvent:insert({'1599174857569a63bbbf2cb2168995381b82bdcd', 'register request'})
chainEvent:insert({'392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02', 'check ticket'})
chainEvent:insert({'392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02', 'decide'})
chainEvent:insert({'392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02', 'examine casually'})
chainEvent:insert({'392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02', 'register request'})
chainEvent:insert({'392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02', 'reinitiate request'})
chainEvent:insert({'392cb1bb3c44a0c1ab984f9ce4ef139786ba8c02', 'reject request'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'check ticket'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'decide'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'examine casually'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'examine thoroughly'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'register request'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'reinitiate request'})
chainEvent:insert({'767141a2b3fd130bcc5b419bf77b244359974943', 'reject request'})
chainEvent:insert({'7b1abdfcb576c4877eb96ef28328313f3acb470e', 'check ticket'})
chainEvent:insert({'7b1abdfcb576c4877eb96ef28328313f3acb470e', 'decide'})
chainEvent:insert({'7b1abdfcb576c4877eb96ef28328313f3acb470e', 'examine thoroughly'})
chainEvent:insert({'7b1abdfcb576c4877eb96ef28328313f3acb470e', 'register request'})
chainEvent:insert({'7b1abdfcb576c4877eb96ef28328313f3acb470e', 'reject request'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'check ticket'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'decide'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'examine casually'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'examine thoroughly'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'pay compensation'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'register request'})
chainEvent:insert({'a7cefa62c3707b1c71747a2c19f74f8fec6f2d9a', 'reinitiate request'})
chainEvent:insert({'bb9a5ad4eddbb17496957d5e63db463442c8e41f', 'check ticket'})
chainEvent:insert({'bb9a5ad4eddbb17496957d5e63db463442c8e41f', 'decide'})
chainEvent:insert({'bb9a5ad4eddbb17496957d5e63db463442c8e41f', 'examine casually'})
chainEvent:insert({'bb9a5ad4eddbb17496957d5e63db463442c8e41f', 'pay compensation'})
chainEvent:insert({'bb9a5ad4eddbb17496957d5e63db463442c8e41f', 'register request'})
chainEvent:insert({'cd2e9d851b51e0ce0796db2f30ec315934a3cf9a', 'check ticket'})
chainEvent:insert({'cd2e9d851b51e0ce0796db2f30ec315934a3cf9a', 'decide'})
chainEvent:insert({'cd2e9d851b51e0ce0796db2f30ec315934a3cf9a', 'examine casually'})
chainEvent:insert({'cd2e9d851b51e0ce0796db2f30ec315934a3cf9a', 'pay compensation'})
chainEvent:insert({'cd2e9d851b51e0ce0796db2f30ec315934a3cf9a', 'register request'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'check ticket'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'decide'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'examine casually'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'examine thoroughly'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'pay compensation'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'register request'})
chainEvent:insert({'e818048b6dab4dd6e3de09b075b2ef2dfb9b6c5f', 'reinitiate request'})
chainEvent:insert({'f311956d23b2bb7193f60b87e195171b8178d666', 'decide'})
chainEvent:insert({'f311956d23b2bb7193f60b87e195171b8178d666', 'examine thoroughly'})
chainEvent:insert({'f311956d23b2bb7193f60b87e195171b8178d666', 'pay compensation'})
chainEvent:insert({'f311956d23b2bb7193f60b87e195171b8178d666', 'register request'})


box.schema.func.create('EVENT_NAMES', {
    language = 'LUA',
    returns = 'string',
    body = [[
            function(chainHash)
                local names = { }
                for _, row in box.space.chainEvent:pairs(chainHash) do
                    table.insert(names, row[2])
                end

                return table.concat(names, ", ")
            end
        ]],
    is_sandboxed = false,
    param_list = { "string" },
    exports = { 'LUA', 'SQL' },
    is_deterministic = false,
    if_not_exists = true
})
box.schema.role.grant("public", "execute", "function", "EVENT_NAMES",
    { if_not_exists = true })


res, err = box.execute([[
  select
    "chain"."hash"
  from "chain"
  where "chain"."modelUid" = ?
    and "chain"."percent" <= ?
  order by "chain"."percent" desc;]],
    { "85777b8f-35ac-40be-a733-647005ee4cfa", 100 })

if err ~= nil then
    error(err)
end

log.info("RESULT 1: ")
log.info(res)

assert(#res.rows == 10)


res, err = box.execute([[
  select
    "chain"."hash",
    EVENT_NAMES("chain"."hash") as "eventNames"
  from "chain"
  where "chain"."modelUid" = ?
    and "chain"."percent" <= ?
  order by "chain"."percent";]],
    { "85777b8f-35ac-40be-a733-647005ee4cfa", 100 })

if err ~= nil then
    error(err)
end

log.info("RESULT 2: ")
log.info(res)

assert(#res.rows == 10)


res, err = box.execute([[
  select
    "chain"."hash",
    EVENT_NAMES("chain"."hash") as "eventNames"
  from "chain"
  where "chain"."percent" <= ?
  order by "chain"."percent" desc;]],
    { 100 })

if err ~= nil then
    error(err)
end

log.info("RESULT 3: ")
log.info(res)

assert(#res.rows == 10)


local res, err = box.execute([[
  select
    "chain"."hash",
    EVENT_NAMES("chain"."hash") as "eventNames"
  from "chain"
  where "chain"."modelUid" = ?
    and "chain"."percent" <= ?
  order by "chain"."percent" desc;]],
    { "85777b8f-35ac-40be-a733-647005ee4cfa", 100 })

if err ~= nil then
    error(err)
end

log.info("RESULT 4: ")
log.info(res)

assert(#res.rows == 10)

And we get this error on the last line:

LuajitError: ./run.lua:254: assertion failed!
@kyukhin kyukhin added bug Something isn't working sql labels Oct 16, 2020
@ImeevMA
Copy link
Collaborator

ImeevMA commented Oct 16, 2020

I looked at this reproducer and found another assert on debug.

Decreased reproducer a bit:

box.cfg{}
box.execute([[CREATE TABLE t1 (b STRING PRIMARY KEY)]])
box.execute([[INSERT INTO t1 VALUES ('aaaaaaaaaaaa')]])
box.schema.func.create('EVENT_NAMES', {
    language = 'LUA',
    returns = 'string',
    body = [[function(x) return box.space.T1:select()[1][1] end]],
    is_sandboxed = false,
    param_list = { "string" },
    exports = { 'LUA', 'SQL' },
    is_deterministic = false,
    if_not_exists = true
})

box.execute([[select EVENT_NAMES(t1.b) from t1 where t1.b = ? and t1.b <= ? order by t1.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"})

Result:

tarantool> box.execute([[select EVENT_NAMES(t1.b) from t1 where t1.b = ? and t1.b <= ? order by t1.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"})
tarantool: /home/mergen/work/dev/src/lib/small/small/region.c:75: region_truncate: Assertion `region_used(region) >= used' failed.
Aborted (core dumped)

I think after we fix this assert the issue also will be fixed.

@ImeevMA ImeevMA self-assigned this Oct 16, 2020
@kyukhin kyukhin added this to the 2.5.3 milestone Nov 2, 2020
ImeevMA added a commit that referenced this issue Nov 2, 2020
@ImeevMA
sql: do not reset region on select
de3a354 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427
@Gerold103 Gerold103 mentioned this issue Nov 4, 2020
Closed
ImeevMA added a commit that referenced this issue Dec 16, 2020
@ImeevMA
sql: do not reset region on select
adc5b10 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427
ImeevMA added a commit that referenced this issue Dec 22, 2020
@ImeevMA
sql: do not reset region on select
c00107e 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427
ImeevMA added a commit that referenced this issue Dec 23, 2020
@ImeevMA
sql: do not reset region on select
265e8f7 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427
Korablev77 pushed a commit that referenced this issue Dec 24, 2020
@ImeevMA @Korablev77
sql: do not reset region on select
6137608 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427

(cherry picked from commit d0d668f)
Korablev77 pushed a commit that referenced this issue Dec 24, 2020
@ImeevMA @Korablev77
sql: do not reset region on select
fc60a36 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427

(cherry picked from commit d0d668f)
ligurio pushed a commit that referenced this issue Dec 25, 2020
@ImeevMA @ligurio
sql: do not reset region on select
ba0b257 
Prior to this patch, region on fiber was reset during select(), get(),
count(), max(), or min(). This would result in an error if one of these
operations was used in a user-defined function in SQL. After this patch,
these functions truncate region instead of resetting it.

Closes #5427
@Totktonada Totktonada mentioned this issue Dec 27, 2020
Closed
@void234 void234 mentioned this issue Dec 28, 2020
Closed
Totktonada pushed a commit that referenced this issue Dec 28, 2020
@ImeevMA @Totktonada
box: do not reset region on select
96916f8 
A read only transaction should not invalidate the fiber region.

The problem was found in context of calling a Lua function from SQL on
tarantool 2.* (#5427).

Another problem (#5623) was fixed by this commit on tarantool 2.*. Since
2.2.0-469-g707e58a3f ('box: rework box_lua_{call, eval} to use input
port') arguments for a C stored procedure is allocated on a fiber
region. The region is truncated at the end of the call. When the
procedure performs a read only transaction, fiber_gc() may be called
and so the region_truncate() call will fail on assertion (on Debug
build).

Despite that all found cases are specific for tarantoo 2.*, it is
possible to be hit by the problem on 1.10 as well. It is enough to
allocate something on a region (explicitly or implicitly) and perform
any read only transaction.

So it looks worthful to backport the fix to 1.10 as well.

(cherry picked from commit d0d668f)

The original commit message:

 | sql: do not reset region on select
 |
 | Prior to this patch, region on fiber was reset during select(), get(),
 | count(), max(), or min(). This would result in an error if one of these
 | operations was used in a user-defined function in SQL. After this patch,
 | these functions truncate region instead of resetting it.
 |
 | Closes #5427
Totktonada pushed a commit that referenced this issue Dec 29, 2020
@ImeevMA @Totktonada
box: do not reset region on select
59789e7 
A read only transaction should not invalidate the fiber region.

The problem was found in context of calling a Lua function from SQL on
tarantool 2.* (#5427).

Another problem (#5623) was fixed by this commit on tarantool 2.*. Since
2.2.0-469-g707e58a3f ('box: rework box_lua_{call, eval} to use input
port') arguments for a C stored procedure is allocated on a fiber
region. The region is truncated at the end of the call. When the
procedure performs a read only transaction, fiber_gc() may be called
and so region_truncate() will fail on assertion (on Debug build).

Despite that all found cases are specific for tarantool 2.*, it is
possible to be hit by the problem on 1.10 as well. It is enough to
allocate something on a region (explicitly or implicitly) and perform a
read only transaction.

So it looks worthful to backport the fix to 1.10 as well.

(cherry picked from commit d0d668f)

The original commit message:

 | sql: do not reset region on select
 |
 | Prior to this patch, region on fiber was reset during select(), get(),
 | count(), max(), or min(). This would result in an error if one of these
 | operations was used in a user-defined function in SQL. After this patch,
 | these functions truncate region instead of resetting it.
 |
 | Closes #5427
Totktonada pushed a commit that referenced this issue Dec 29, 2020
@ImeevMA @Totktonada
box: do not reset region on select
107ad3f 
A read only transaction should not invalidate the fiber region.

The problem was found in context of calling a Lua function from SQL on
tarantool 2.* (#5427).

Another problem (#5623) was fixed by this commit on tarantool 2.*. Since
2.2.0-469-g707e58a3f ('box: rework box_lua_{call, eval} to use input
port') arguments for a C stored procedure is allocated on a fiber
region. The region is truncated at the end of the call. When the
procedure performs a read only transaction, fiber_gc() may be called
and so region_truncate() will fail on assertion (on Debug build).

Despite that all found cases are specific for tarantool 2.*, it is
possible to be hit by the problem on 1.10 as well. It is enough to
allocate something on a region (explicitly or implicitly) and perform a
read only transaction.

So it looks worthful to backport the fix to 1.10 as well.

(cherry picked from commit d0d668f)

The original commit message:

 | sql: do not reset region on select
 |
 | Prior to this patch, region on fiber was reset during select(), get(),
 | count(), max(), or min(). This would result in an error if one of these
 | operations was used in a user-defined function in SQL. After this patch,
 | these functions truncate region instead of resetting it.
 |
 | Closes #5427
@pgulutzan pgulutzan mentioned this issue Jan 15, 2021
Closed
@igormunkin igormunkin removed the teamL label Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ImeevMA ImeevMA

Labels
bug Something isn't working sql
Projects
None yet
Milestone
2.5.3
Development

No branches or pull requests
4 participants
@ImeevMA @igormunkin @kyukhin @msiomkin