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

scan: fix nil filters #425

Merged
merged 8 commits into from
Mar 20, 2024

Conversation

DifferentialOrange
Copy link
Member

Before this patch, some nil conditions failed with internal filter library build error. This patch fixes this internal error, as well as normalize filters to behave similar to core indexes.

The logic in core Tarantool select is as follows. nil condition in index select is an absence of condition, thus all data is returned disregarding the condition (condition may affect the order). box.NULL condition is a condition for the null value -- in case of EQ, only records with null index value are returned, in case of GT, all non-null values are returned since nulls are in the beginning of an index and so on. nils and box.NULLs in tuple are both satisfy box.NULL equity.

After this patch, nil filter condition is treated as no condition. This is a breaking change since conditions for '>' and '<' operator with nil operand had resulted with empty response before this patch. But since it was inconsistent with scanning index conditions and wasn't intentional, we change it here.

Closes #422

@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-422-nil-conditions branch 2 times, most recently from ee1de7a to ffa15ea Compare March 15, 2024 14:11
There seem to be no reason to determine features on request instead of
require since this operation is cheap.

Part of #422
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-422-nil-conditions branch 3 times, most recently from a5f10cd to 4de7d35 Compare March 18, 2024 15:15
Generate `tarantool_supports_X` utils instead of creating a manual
function for each key with copypaste.

Part of #422
`pcall(require, module)` check is insufficient for external types
since module presence often precedes box format support and index
support for a type. Sometimes the nature of the module may change
between versions: for example, there is a `'uuid'` module in 1.10.x,
but a separate type was introduced only in 2.4.1

Part of #422
Test scenarios which imitate "server fail" leave test cluster
malfunctioning even in cartridge case.

Part of #422
Before this patch, some nil conditions failed with internal filter
library build error. This patch fixes this internal error, as well as
normalize filters to behave similar to core indexes.

The logic in core Tarantool select is as follows. `nil` condition in
index select is an absence of condition, thus all data is returned
disregarding the condition (condition may affect the order). `box.NULL`
condition is a condition for the null value -- in case of EQ,
only records with null index value are returned, in case of GT,
all non-null values are returned since nulls are in the beginning of an
index and so on. `nil`s and `box.NULL`s in tuple are both satisfy
`box.NULL` equity.

After this patch, `nil` filter condition is treated as no condition.
This is a breaking change since conditions for `'>'` and `'<'`
operator with `nil` operand had resulted with empty response
before this patch. But since it was inconsistent with scanning index
conditions and wasn't intentional, we change it here.

Closes #422
Copy link
Contributor

@better0fdead better0fdead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for patch, code looks great, LGTM

@DifferentialOrange DifferentialOrange merged commit 5b05d38 into master Mar 20, 2024
24 checks passed
@DifferentialOrange DifferentialOrange deleted the DifferentialOrange/gh-422-nil-conditions branch March 20, 2024 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle 'wrong symbol )' exception in case of comparision field with nil or {} on nonindexed column
2 participants