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

Tarantool requires fields that actually does not need #5027

Closed
alyapunov opened this issue May 27, 2020 · 2 comments
Closed

Tarantool requires fields that actually does not need #5027

alyapunov opened this issue May 27, 2020 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@alyapunov
Copy link
Contributor

alyapunov commented May 27, 2020

Tarantool version: 2.5.0
Having nullable field, tarantool does not requre the field to be set, but occasionally requires preceding fields.

Example
tarantool> s = box.schema.space.create('test')
---
...

tarantool> i1 = s:create_index('i1', {parts={{1, 'unsigned'}}})
---
...

tarantool> i2 = s:create_index('i2', {parts={{5, 'unsigned', is_nullable=true}}})
---
...

tarantool> s:replace{1}
---
- error: Tuple field 2 required by space format is missing
...

tarantool> s:replace{1, box.NULL}
---
- error: Tuple field 3 required by space format is missing
...

tarantool> s:replace{1, box.NULL, box.NULL}
---
- error: Tuple field 4 required by space format is missing
...

tarantool> s:replace{1, box.NULL, box.NULL, box.NULL}
---
- [1, null, null, null]
...

In this example tarantool does not require field 5 since it's nullable, and that is correct. But if the field 5 is not present I expect tarantool not to require fields 2-4.

@kyukhin kyukhin added the bug Something isn't working label May 28, 2020
@kyukhin
Copy link
Contributor

kyukhin commented May 28, 2020

Reproduced on 2.3 and 2.4 as well. 1.10 seems to be OK.

@kyukhin kyukhin added this to the 2.3.3 milestone May 28, 2020
@PersDep PersDep self-assigned this Jun 11, 2020
PersDep added a commit that referenced this issue Jun 18, 2020
Since e1d3fe8 (tuple format: don't
allow null where array/map is expected) tuple fields are nullable by
default. It seems strange at least in case we have implicit fields in
front of explicit nullable field. Now fields are nullable by default
except arrays & maps due to reasons specified in mentioned commit.

Closes #5027
PersDep added a commit that referenced this issue Jun 23, 2020
Since e1d3fe8 (tuple format: don't
allow null where array/map is expected) tuple fields are nullable by
default. It seems strange at least in case we have implicit fields in
front of explicit nullable field. Now fields are nullable by default
except arrays & maps due to reasons specified in mentioned commit.

Closes #5027
@PersDep
Copy link
Contributor

PersDep commented Jul 12, 2020

Turns out there is one more closely related issue:
Explicitly specifying nullable array field and creating multikey index based on it leads to fail in tuple_raw_multikey_count.
Reproducer:

s = box.schema.space.create('test')
s:format({{name='id'}, {name='data', type='array', is_nullable=true}})
s:create_index('i1', {parts={{1, 'unsigned'}}})
s:create_index('i2', {parts={{field=2, path='[*].key', type='string'}}})
s:replace{1, box.NULL}

Thus we not only need to fix tuple fields nullability issues but also provide suitable restrictions for multikey index.

PersDep added a commit that referenced this issue Jul 12, 2020
Since e1d3fe8 (tuple format: don't
allow null where array/map is expected) tuple fields are non-nullable
by default. It seems strange at least in case we have implicit fields
in front of explicit nullable field. Also it causes incorrect behaviour
in case of using explicitly nullable array/map fields for multikey
index.
Now fields are nullable by default except arrays & maps, as far
as their implicit nullability might break field accessors expectations,
provide confusing error messages and cause incorrect behaviour of
tuple_multikey_count(). In case explicitly nullable array/map fields
are being used for multikey index, clear error message is provided.

Closes #5027
kyukhin pushed a commit that referenced this issue Jul 13, 2020
Since e1d3fe8 (tuple format: don't
allow null where array/map is expected) tuple fields are non-nullable
by default. It seems strange at least in case we have implicit fields
in front of explicit nullable field. Also it causes incorrect behaviour
in case of using explicitly nullable array/map fields for multikey
index.
Now fields are nullable by default except arrays & maps, as far
as their implicit nullability might break field accessors expectations,
provide confusing error messages and cause incorrect behaviour of
tuple_multikey_count(). In case explicitly nullable array/map fields
are being used for multikey index, clear error message is provided.

Closes #5027

(cherry picked from commit 4cf94ef)
kyukhin pushed a commit that referenced this issue Jul 13, 2020
Since e1d3fe8 (tuple format: don't
allow null where array/map is expected) tuple fields are non-nullable
by default. It seems strange at least in case we have implicit fields
in front of explicit nullable field. Also it causes incorrect behaviour
in case of using explicitly nullable array/map fields for multikey
index.
Now fields are nullable by default except arrays & maps, as far
as their implicit nullability might break field accessors expectations,
provide confusing error messages and cause incorrect behaviour of
tuple_multikey_count(). In case explicitly nullable array/map fields
are being used for multikey index, clear error message is provided.

Closes #5027

(cherry picked from commit 4cf94ef)
PersDep added a commit that referenced this issue Jul 24, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was since e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192
PersDep added a commit that referenced this issue Jul 24, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192
PersDep added a commit that referenced this issue Jul 24, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192
PersDep added a commit that referenced this issue Aug 5, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192
PersDep added a commit that referenced this issue Aug 14, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192
kyukhin pushed a commit that referenced this issue Aug 25, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192

(cherry picked from commit bfeb61b)
kyukhin pushed a commit that referenced this issue Aug 25, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192
kyukhin pushed a commit that referenced this issue Aug 25, 2020
Multikey index did not work properly with nullable root field in
tuple_raw_multikey_count(). Now it is fixed and corresponding
restrictions are dropped. This also means that we can drop implicit
nullability update for array/map fields and make all fields nullable
by default, as it was until e1d3fe8
(tuple format: don't allow null where array/map is expected), as far as
default non-nullability itself doesn't solve any real problems while
providing confusing behavior (gh-5027).

Follow-up #5027
Closes #5192

(cherry picked from commit bfeb61b)
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

3 participants