Added function accurateCastOrNull, allow different types inside IN subquery by kitaisreal · Pull Request #16724 · ClickHouse/ClickHouse

kitaisreal · 2020-11-05T19:11:02Z

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Mentioned in #10290.
Mentioned in #10266.

Changelog category (leave one):

New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added function accurateCastOrNull. This closes #10290. Add type conversions in x IN (subquery) expressions. This closes #10266.

kitaisreal · 2020-11-05T19:12:26Z

src/Functions/FunctionsConversion.h

@alexey-milovidov not sure how to properly implement it for types without numeric limits, do we have some places where similar functionality implemented ? Thanks.

The most simple is to implement numeric limits for bigints.

kitaisreal · 2020-11-15T10:31:52Z

src/Core/Settings.cpp

@alexey-milovidov latest clang warning

Documents/ClickhouseBrowse/src/Core/Settings.cpp:20:1: error: stack frame size of 81128 bytes in lambda expression [-Werror,-Wframe-larger-than=] IMPLEMENT_SETTINGS_TRAITS(SettingsTraits, LIST_OF_SETTINGS) ^ Documents/ClickhouseBrowse/src/Core/BaseSettings.h:817:46: note: expanded from macro 'IMPLEMENT_SETTINGS_TRAITS' static const Accessor the_instance = [] \

Documents/ClickhouseBrowse/build$ clang++-11 --version Ubuntu clang version 11.0.1-++20201103062930+ef4ffcafbb2-1~exp1~20201103053545.129

Ok. Frame size limit is currently just a precaution.

kitaisreal · 2020-12-02T10:11:18Z

@alexey-milovidov make test implementation of allowing different types inside in subquery. Please check 01600_select_in_different_types.sql and Set.cpp for a reference.

alexey-milovidov · 2020-12-02T10:23:24Z

tests/queries/0_stateless/01600_select_in_different_types.sql

It's ok to keep.

It's more aligned with the behaviour of standard SQL where weak typing is used.
We don't like weak typing :) But we tend to make our SQL more compatible with standard (through some settings or without). If this behaviour is "for free", let's keep it without introducing any settings.

@alexey-milovidov It is for free for cases where we insert data in set from subselect, but when we make set from collection during AST visiting, we use convertFieldToType and it will throw exception if types does not match.

Example: SELECT '1' IN (1); will throw exception.

It does not make a lot of changes to update this behaviour. Should we fix it ?

It looks inconsistent but let's keep it as is for now.

alexey-milovidov · 2020-12-02T12:09:30Z

src/Core/AccurateComparison.h

What if we convert Float64's inf to Float32's inf or vice versa?

Also https://en.cppreference.com/w/cpp/types/numeric_limits/lowest ?

Will investigate floating point types conversion, seems like there is are a bunch of problems.

Related to infinite conversions we can add special case above in addition to isNan.

BTW, loss of precision due to Float64 -> Float32 conversion is Ok. For example,

toFloat32(0.1) IN (SELECT 0.1) should work.

So, we can assume all the range of [Float32_lowest, Float32_max] in Float64 to be representable in Float32.

(actually I'm not 100% sure about it)

Updated. As discussed does not create specialization for case like toFloat32(0.1) IN (SELECT 0.1).

alexey-milovidov · 2020-12-02T12:13:37Z

src/Core/AccurateComparison.h

Also what's about the case of conersion 1.5 to integer type (should return false)?

PS. Exact equality is Ok, so 1.000...1 also not equals to 1.

Agreed equalsOp is required.

alexey-milovidov · 2020-12-02T12:38:42Z

src/Interpreters/Set.cpp

This is Ok :)

BTW we also have obscure setting transform_null_in.

We can disable conversions (and resort to strict type checking) when it's enabled.

Updated as we discussed, adding additional function accurateCast.

alexey-milovidov · 2020-12-02T12:39:25Z

src/Interpreters/Set.cpp

Remove before merge...

1. Added accurateCast function. 2. Use accurateCast in Set during execute. 3. Added accurateCast tests. 4. Updated select_in_different_types tests.

alexey-milovidov · 2021-01-31T04:27:56Z

@kitaisreal This is somehow related: https://clickhouse-test-reports.s3.yandex.net/19677/3b6395fbb774c132273083f4f84a4f5a34c12866/fuzzer_debug/server.log

robot-clickhouse added doc-alert pr-feature Pull request with new product feature labels Nov 5, 2020

kitaisreal commented Nov 5, 2020

View reviewed changes

alexey-milovidov added the can be tested label Nov 5, 2020

kitaisreal force-pushed the added-function-accurate-cast-or-null branch 4 times, most recently from e8c11fe to cddff6b Compare November 15, 2020 10:04

kitaisreal commented Nov 15, 2020

View reviewed changes

kitaisreal requested a review from alexey-milovidov November 15, 2020 20:05

alexey-milovidov self-assigned this Nov 24, 2020

kitaisreal force-pushed the added-function-accurate-cast-or-null branch 3 times, most recently from b39d199 to ed54023 Compare December 1, 2020 20:12

kitaisreal changed the title ~~Added function accurateCastOrNull~~ Added function accurateCastOrNull, allow different types inside IN subquery Dec 2, 2020

alexey-milovidov reviewed Dec 2, 2020

View reviewed changes

kitaisreal force-pushed the added-function-accurate-cast-or-null branch 2 times, most recently from 91f35e9 to 4bba2d3 Compare December 5, 2020 14:55

kitaisreal requested a review from alexey-milovidov December 5, 2020 16:05

kitaisreal force-pushed the added-function-accurate-cast-or-null branch 2 times, most recently from deb60ce to efe0525 Compare December 11, 2020 13:51

kitaisreal added 4 commits December 14, 2020 22:12

Added function accurate cast or null

d636e16

Added numeric limits for UInt128

4cd5025

Fixed setting null value in vector. Updated tests.

baad78d

Updated documentation

476fb3b

kitaisreal and others added 16 commits December 14, 2020 22:12

Added Decimal support

db5d094

Fixed style issues

4435642

Fixed clang compile issues

7b66dc3

Fixed compile issues

dd072a2

Added const to arguments

0dac432

Fixed compile issues

dff31e8

Allow different types inside IN subquery

f4b8e8e

Added equality comparsion

6f4bc77

Updated usage of different types during IN query

0464859

1. Added accurateCast function. 2. Use accurateCast in Set during execute. 3. Added accurateCast tests. 4. Updated select_in_different_types tests.

Fixed compile issues

18dc118

Fixed Set execute for columns that cannot be inside Nullable

0b565b6

Fixed clang build issue

6873b13

Update castColumn.h

26e5822

Update castColumn.h

e945821

Updated non-finite floating conversion into int

16a30eb

Fixed tests

17b43ca

kitaisreal force-pushed the added-function-accurate-cast-or-null branch from efe0525 to 17b43ca Compare December 14, 2020 19:12

kitaisreal and others added 3 commits December 15, 2020 21:54

Added tryParseDecimal functions

51f53ce

Fixed tests

4e3cc60

Update DecimalFunctions.h

aec8203

alexey-milovidov approved these changes Dec 15, 2020

View reviewed changes

kitaisreal added 2 commits December 17, 2020 21:32

Fixed compilation timeout

9aaa2f9

Fixed ya.make

0c04a26

alexey-milovidov approved these changes Dec 17, 2020

View reviewed changes

alexey-milovidov merged commit 6fc3ca8 into ClickHouse:master Dec 17, 2020

alexey-milovidov mentioned this pull request Jan 11, 2021

Roadmap 2021 (discussion) #17623

Closed

AnaUvarova mentioned this pull request Jan 25, 2021

DOCSUP-5822: Documenting the accurateCastOrNull function #19561

Closed

ka1bi4 mentioned this pull request Feb 2, 2021

DOCSUP-5822: Document the accurateCastOrNull function. #19965

Merged

Conversation

kitaisreal commented Nov 5, 2020 • edited by alexey-milovidov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kitaisreal commented Dec 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov commented Jan 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kitaisreal commented Nov 5, 2020 •

edited by alexey-milovidov

Loading

alexey-milovidov Dec 2, 2020 •

edited

Loading

alexey-milovidov Dec 2, 2020 •

edited

Loading