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

has function doesn't work corectly with Nullable strings #2115

Closed
silviucpp opened this issue Mar 26, 2018 · 8 comments
Closed

has function doesn't work corectly with Nullable strings #2115

silviucpp opened this issue Mar 26, 2018 · 8 comments
Assignees
Labels
bug Confirmed user-visible misbehaviour in official release

Comments

@silviucpp
Copy link
Contributor

silviucpp commented Mar 26, 2018

has functions seems is not working correctly when we have an array of nullable strings:

I saw that using Array(Nullable(Uint8)) is working correctly . I didn't tested with more data types.

CREATE TABLE test_has_function(arr Array(Nullable(String))) ENGINE = Memory;
INSERT INTO test_has_function(arr) values ([null, 'str1', 'str2']),(['str1', 'str2']), ([]), ([]);
SELECT arr, has(`arr`, 'str1') FROM test_has_function;

[null,'str1','str2']  0 (WRONG - should be 1)
['str1','str2']       1 (OK)
[]                    0 (OK)
[]                    0 (OK)
@silviucpp
Copy link
Contributor Author

SELECT has([null, 'str1', 'str2'], 'str1') works as well.

@silviucpp
Copy link
Contributor Author

Hello @alexey-milovidov ,

I'm trying to investigate this issue and I have a question: For the example I provided above what branch in FunctionArray it's expected to be executed starting from line 855:

Currently the second one is executed, but I have a feeling that first one is the proper one and item_arg->onlyNull() should evaluate to true and not false for this case.

if (item_arg->onlyNull())
	ArrayIndexStringNullImpl<IndexConv>::vector_const(col_nested->getChars(), col_array->getOffsets(),
		col_nested->getOffsets(), col_res->getData(), null_map_data);
else if (const auto item_arg_const = checkAndGetColumnConstStringOrFixedString(item_arg))
	ArrayIndexStringImpl<IndexConv>::vector_const(col_nested->getChars(), col_array->getOffsets(),
		col_nested->getOffsets(), item_arg_const->getValue<String>(), col_res->getData(),
		null_map_data);
else if (const auto item_arg_vector = checkAndGetColumn<ColumnString>(item_arg))
	ArrayIndexStringImpl<IndexConv>::vector_vector(col_nested->getChars(), col_array->getOffsets(),
		col_nested->getOffsets(), item_arg_vector->getChars(), item_arg_vector->getOffsets(),
		col_res->getData(), null_map_data, null_map_item);

I'm trying to realize if I'm on the good track with debugging

Silviu

@silviucpp
Copy link
Contributor Author

@alexey-milovidov you consider this a valid bug ?

@alexey-milovidov alexey-milovidov added the bug Confirmed user-visible misbehaviour in official release label Jul 6, 2018
@alexey-milovidov alexey-milovidov self-assigned this Jul 6, 2018
@alexey-milovidov
Copy link
Member

Yes, it's a bug. I'll try to investigate.

@alexey-milovidov
Copy link
Member

item_arg->onlyNull() should evaluate to true and not false for this case

It's for the case when you write arrayHas([...], NULL)

@silviucpp
Copy link
Contributor Author

Ok, so problem is in another place.

@alexey-milovidov
Copy link
Member

The code is just plain wrong (no magic).

@alexey-milovidov
Copy link
Member

Fix is in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed user-visible misbehaviour in official release
Projects
None yet
Development

No branches or pull requests

2 participants