Skip to content

Remove select with custom merger #365

Open
@DifferentialOrange

Description

@DifferentialOrange

crud uses tuple-merger for select. Built-in merger is provided for the following versions

crud/crud/common/utils.lua

Lines 589 to 599 in 2d3d479

enabled_tarantool_features.builtin_merger = is_version_ge(major, minor, patch, suffix,
2, 6, 0, nil)
or is_version_in_range(major, minor, patch, suffix,
2, 5, 1, nil,
2, 5, math.huge, nil)
or is_version_in_range(major, minor, patch, suffix,
2, 4, 2, nil,
2, 4, math.huge, nil)
or is_version_in_range(major, minor, patch, suffix,
2, 3, 3, nil,
2, 3, math.huge, nil)

and external merger can be provided for the following versions

crud/crud/common/utils.lua

Lines 607 to 620 in 2d3d479

enabled_tarantool_features.external_merger = is_version_ge(major, minor, patch, suffix,
2, 7, 0, nil)
or is_version_in_range(major, minor, patch, suffix,
2, 6, 1, nil,
2, 6, math.huge, nil)
or is_version_in_range(major, minor, patch, suffix,
2, 5, 2, nil,
2, 5, math.huge, nil)
or is_version_in_range(major, minor, patch, suffix,
2, 4, 3, nil,
2, 4, math.huge, nil)
or is_version_in_range(major, minor, patch, suffix,
1, 10, 8, nil,
1, 10, math.huge, nil)

There is a separate select implementation without merger for older versions without internal merger support (<= 1.10.7, == 2.5.0, >= 2.4.0, <= 2.4.1, >= 2, <= 2.3.2) and without internal merger if external not provided (< 2.0.0): select_old.

All versions that require select_old are out of support for a long time: it covers <= 1.10.7 and >=2, <= 2.5.2 releases. There is also the >= 1.10.8, <= 1.10.15version range withselect_old` support.

Supporting two select implementation is really bothersome. A lot of time spent to update a code which doesn't seems relevant anymore. I propose to remove this code.

The course of action is as follows.

  1. Ensure that it is fine to drop support of Tarantool releases <= 1.10.7 >=2, <= 2.5.2.
  2. Ensure that it is fine to require all users of Tarantool releases >= 1.10.8, <= 1.10.15to installtuple-merger` as well. (We already seem to do that in CMake:

    crud/CMakeLists.txt

    Lines 105 to 107 in 2d3d479

    execute_process(
    COMMAND bash "-c" "tarantoolctl rocks install tuple-merger 0.0.2"
    )
    ).
  3. Migrate tuple-merger dependency from CMake to rockspec, as well as tuple-keydef one.
  4. Remove select_old code and compatibility layers in code. Remove related tests and CI presets. Rework "if there is a merger" condition to strict assertion.
  5. Tag a new module major version.
  6. File a ticket related to removing tuple-merger dependency in the future: after Tarantool releases >= 1.10.8, <= 1.10.15` support drop there won't be any need for external merger in supported versions.

Metadata

Metadata

Labels

code healthImprove code readability, simplify maintenance and so onquestionFurther information is requested

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions