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.

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

get_index_for_condition should take into account the partial index #231

Open
R-omk opened this issue Nov 2, 2021 · 5 comments
Open

get_index_for_condition should take into account the partial index #231

R-omk opened this issue Nov 2, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@R-omk
Copy link

R-omk commented Nov 2, 2021

Since exclude_null is implemented choosing the wrong index may affect the result.

@Totktonada Totktonada added bug Something isn't working teamE labels Nov 29, 2021
@DifferentialOrange DifferentialOrange removed their assignment Jun 13, 2023
@DifferentialOrange
Copy link
Member

DifferentialOrange commented Jun 14, 2023

To be honest, I don't get what issue is described here, especially considering exclude_null comment. How exactly we should take partial index into account? We already pick multipart index if there is a first field in condition specified. In other cases, there is fullscan either way so it's not like multipart index is more preferable than any other one.

It seems that this issue is likely to be closed if we support specifying scan_index for issue #346 . If it is so, I'll close this issue in favor of #346

@R-omk
Copy link
Author

R-omk commented Jun 14, 2023

pick multipart index if there is a first field in condition specified

that's exactly the problem, it cannot use a partial index because if any other field is null, then the record will simply be lost.

Before closing this ticket it is better to make a draft with tests covering this case

@DifferentialOrange
Copy link
Member

that's exactly the problem, it cannot use a partial index because if any other field is null, then the record will simply be lost.

Can you provide an example of the request and the description of expected and actual behavior? If relevant, with single instance request comparison.

@R-omk
Copy link
Author

R-omk commented Jun 14, 2023

Ughhh...

let's run command from Readme -> Quickstart

$ git clone https://github.com/tarantool/crud.git
$ cd crud
$ tarantoolctl rocks make
$ ./doc/playground.lua
tarantool> crud.select('customers', {{'<=', 'age', 35}}, {first = 10})
tarantool> crud.select('developers', nil, {first = 6})

try to find Elizabeth

...

tarantool> crud.select('customers', {{'=', 'name', 'Elizabeth'}}, {first = 10})
---
- metadata: [{'name': 'id', 'type': 'unsigned'}, {'name': 'bucket_id', 'type': 'unsigned'},
    {'name': 'name', 'type': 'string'}, {'name': 'age', 'type': 'number'}, {'type': 'number',
      'name': 'nul', 'is_nullable': true}]
  rows:
  - [1, 477, 'Elizabeth', 12]
  - [7, 693, 'Elizabeth', 18]
- null
...

ok, the records are really exists

let's add nullable field and add an partial index, it shouldn't break anything, right?


box.space.customers:alter({name = 'customers', format = {
    {name = 'id', type = 'unsigned'},
    {name = 'bucket_id', type = 'unsigned'},
    {name = 'name', type = 'string'},
    {name = 'age', type = 'number'},
    {name = 'nul', type = 'number', is_nullable=true},
}})

box.space.customers:create_index('partial1', {parts = {{'name', 'string', is_nullable=true,exclude_null=true},{'nul', 'number', is_nullable=true,exclude_null=true}}})

look for Elizabeth again

tarantool> crud.select('customers', {{'=', 'name', 'Elizabeth'}}, {first = 10})
---
- metadata: [{'name': 'id', 'type': 'unsigned'}, {'name': 'bucket_id', 'type': 'unsigned'},
    {'name': 'name', 'type': 'string'}, {'name': 'age', 'type': 'number'}, {'type': 'number',
      'name': 'nul', 'is_nullable': true}]
  rows: []
- null
...

oops, what happened?

drop index and retry search

tarantool> box.space.customers.index.partial1:drop()
---
...

tarantool> crud.select('customers', {{'=', 'name', 'Elizabeth'}}, {first = 10})
---
- metadata: [{'name': 'id', 'type': 'unsigned'}, {'name': 'bucket_id', 'type': 'unsigned'},
    {'name': 'name', 'type': 'string'}, {'name': 'age', 'type': 'number'}, {'type': 'number',
      'name': 'nul', 'is_nullable': true}]
  rows:
  - [1, 477, 'Elizabeth', 12]
  - [7, 693, 'Elizabeth', 18]
- null
...

Sigh... looks like it's fixed...

@DifferentialOrange
Copy link
Member

Thank you, now I'll get what this ticket is about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants