Support new _index format #164
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
Totktonada
reviewed
Jul 15, 2020
|
I looked briefly into the patchset. Except several coments above I have a couple of general comments (below). I would add some test cases within commits, where a new functionality is implemented. Say, 'update processing the field type' surely adds the new functionality and it would be more clear if a test case will show what exactly becomes supported. I would add 'build', 'refactoring' prefixes to explicitly mark commits that does not chamge any behaviour. I would maybe also introduce 'schema' prefix, it may be convenient. I didn't look thoroughly into the patch, so sorry that my comments are mostly formatting nitpickings and questions. |
LeonidVas
force-pushed
the
lvasiliev/gh-151-support-new-index-format
branch
3 times, most recently
from
919235c to
86c58a6
Compare
Totktonada
reviewed
Jul 17, 2020
Before the patch, the default compiler compilation mode is used (including C89, version dependent). But support the C89 dialect in 2020 is strange. Accordingly, set C99 as preffered.
There is serious "inline disease" in the code. It seems that "inline" is set to all static function. The patch clears the tarantool_schema.c file.
Alignment of structure members to "natural" address boundaries will cause to a 4-byte gap between field_name and field_name_len on 64-bit platform. If the members will be swapped, the gap is gone.
To simplify adding further changes, move decoding index parts to a separate function and update it in accordance with the decoding function from tarantool. Part of #151
Add collation and is_nullable opts to schema_field_value. Part of #151
Update processing the field type in accordance with the same in tarantool. Part of #151
LeonidVas
force-pushed
the
lvasiliev/gh-151-support-new-index-format
branch
from
86c58a6 to
6089295
Compare
After tarantool-1.7.5-153-g1651fc9be the new _index format was introduced. We should support it to fetch a schema from a tarantool-1.7.5.153+ instance. When an index parts do not use parameters except name and type the index info is stored in the old format in _index system space. When an index part uses is_nullable or collation parameter, then the new format will be used. Close #151
Doesn't change user visible behaviour. * Add support is_nullable to parsing _space system space. * Replace string comparation. It's no always correct and safe to use memcmp to compare a null-terminated string with a length specified string, because it doesn't check the end of null-terminated string (like strncmp) which may be less.
Added support for the new _index format. The time is come to enable tests for tarantool-2.2.
LeonidVas
force-pushed
the
lvasiliev/gh-151-support-new-index-format
branch
from
6089295 to
7f8bc60
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support new _index system space format.
Done some fixes and improvements.
Fixes #151