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

join_use_nulls on LowCardinality type does not work #15101

Closed
mcgrawia opened this issue Sep 21, 2020 · 7 comments · Fixed by #23237
Closed

join_use_nulls on LowCardinality type does not work #15101

mcgrawia opened this issue Sep 21, 2020 · 7 comments · Fixed by #23237
Assignees
Labels
bug Confirmed user-visible misbehaviour in official release comp-lowcardinality LowCardinality data type feature

Comments

@mcgrawia
Copy link

Describe the bug
When I use the set join_use_nulls = 1; setting, LowCardinality columns still show up with the default "" value.

How to reproduce

  • Which ClickHouse server version to use
    20.6.5.8

  • Queries to run that lead to unexpected result

create table left (
    id integer
) ENGINE = Memory();

create table right (
    id integer,
    batch LowCardinality(String)
) ENGINE = Memory();

insert into left (id) select * from system.numbers limit 10;
insert into right (id, batch) values (0, 'test'), (1, 'test'), (2, 'test');

set join_use_nulls = 1;
select * from left left outer join right on left.id = right.id;

Results:

0	0     test
1	1     test
2	2     test
3	null  ""
4	null  ""
5	null  ""
6	null  ""
7	null  ""
8	null  ""
9	null  ""

Expected behavior

0	0       test 
1	1       test 
2	2       test 
3	null	null 
4	null	null 
5	null	null 
6	null	null 
7	null	null 
8	null	null 
9	null	null 
@mcgrawia mcgrawia added the bug Confirmed user-visible misbehaviour in official release label Sep 21, 2020
@den-crane
Copy link
Contributor

WA:

SELECT *
FROM left
LEFT JOIN
(
    SELECT
        id,
        cast(batch, 'LowCardinality(Nullable(String))')
    FROM right
) AS right ON left.id = right.id

┌─id─┬─right.id─┬─cast(batch, 'LowCardinality(Nullable(String))')─┐
│  0 │        0 │ test                                            │
│  1 │        1 │ test                                            │
│  2 │        2 │ test                                            │
│  3 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
│  4 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
│  5 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
│  6 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
│  7 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
│  8 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
│  9 │     ᴺᵁᴸᴸ │ ᴺᵁᴸᴸ                                            │
└────┴──────────┴─────────────────────────────────────────────────┘

@4ertus2 4ertus2 added comp-lowcardinality LowCardinality data type feature labels Sep 21, 2020
@4ertus2
Copy link
Contributor

4ertus2 commented Sep 21, 2020

Some of types are not allowed inside Nullable yet. LC - one of them. There's also tuples and arrays. You may check it with

SELECT toTypeName(toNullable(something))

For LC it returns

SELECT toTypeName(toNullable(toLowCardinality('c')))

┌─toTypeName(toNullable(toLowCardinality('c')))─┐
│ LowCardinality(Nullable(String))              │
└───────────────────────────────────────────────┘

Unfortunately it's not the same as Nullable(LowCardianlity(String)) which is not supported. So JOIN cannot change it's nullability the same way as all other types.

@alexey-milovidov
Copy link
Member

@4ertus2 but LowCardinality of Nullable is Ok.

@alexey-milovidov
Copy link
Member

Works perfectly on version 20.13-testing.
I will add a test.

@alexey-milovidov
Copy link
Member

@KochetovNicolai It does not work.

@alexey-milovidov
Copy link
Member

@KochetovNicolai it still does not work.

@vdimir
Copy link
Member

vdimir commented Apr 17, 2021

LowCardinality of Nullable is Ok.

@alexey-milovidov

Now canBeInsideNullable is false for LowCardinality(String). Enabling it in ColumnLowCardinality.h and DataTypeLowCardinality.h makes this query work, I'll watch on other tests in CI #23220

UPD: this is about Nullable(LowCardinality(T)) but we need LowCardinality(Nullable(T)), trying in #23237

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 comp-lowcardinality LowCardinality data type feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants