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

Short circuit function evaluation is not applied to constant in conversion functions #37038

Open
csavelief opened this issue May 9, 2022 · 7 comments
Assignees

Comments

@csavelief
Copy link

csavelief commented May 9, 2022

Hi,

First, I am using ClickHouse 21.10.5.3 according to the following query:

SELECT version()

Then, I have an inconsistent result when executing this query:

SELECT if(
  startsWith('-1671.45', '.'),
  toFloat64(concat('0', '-1671.45')),
  toFloat64('-1671.45')
)

Because

SELECT startsWith('-1671.45', '.')

returns 0, I expect the toFloat64('-1671.45') to be executed instead of toFloat64(concat('0', '-1671.45')).

Did I make a mistake somewhere?

Best,

C.

@csavelief csavelief added the potential bug To be reviewed by developers and confirmed/rejected. label May 9, 2022
@UnamedRus
Copy link
Contributor

BTW why do you need it at all?

SELECT toFloat64('.45') AS res

Query id: 5d63b722-8a4b-4bc9-941d-b2ee0fa4aa24

┌──res─┐
│ 0.45 │
└──────┘

@vdimir
Copy link
Member

vdimir commented May 9, 2022

With con-constant column it works correctly (on master):


SELECT if(startsWith('-1671.45', '.'), toFloat64(materialize(concat('0', '-1671.45'))), toFloat64('-1671.45'))


┌─if(startsWith('-1671.45', '.'), toFloat64(materialize(concat('0', '-1671.45'))), toFloat64('-1671.45'))─┐
│                                                                                                -1671.45 │
└─────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Ref #23367, cc @Avogar

@Avogar Avogar self-assigned this May 9, 2022
@vdimir vdimir added unexpected behaviour and removed potential bug To be reviewed by developers and confirmed/rejected. labels May 9, 2022
@vdimir vdimir changed the title Strange if behavior Short circuit function evaluation is not applied to constant May 9, 2022
@Avogar
Copy link
Member

Avogar commented May 9, 2022

It's related to default implementation for constants in conversion functions and constant folding while query analysis. Will look is it possible to disable it for short-circuit case

@Avogar Avogar changed the title Short circuit function evaluation is not applied to constant Short circuit function evaluation is not applied to constant in conversion functions May 9, 2022
@den-crane
Copy link
Contributor

@csavelief

WA: toFloat64OrZero

SELECT if(startsWith('-1671.45', '.'), toFloat64OrZero(concat('0', '-1671.45')), toFloat64OrZero('-1671.45'))

Query id: 9994cec3-d8ce-4693-940b-c0de604a8675

┌─if(startsWith('-1671.45', '.'), toFloat64OrZero(concat('0', '-1671.45')), toFloat64OrZero('-1671.45'))─┐
│                                                                                               -1671.45 │
└────────────────────────────────────────────────────────────────────────────────────────────────────────┘

@csavelief
Copy link
Author

BTW why do you need it at all?

SELECT toFloat64('.45') AS res

Query id: 5d63b722-8a4b-4bc9-941d-b2ee0fa4aa24

┌──res─┐
│ 0.45 │
└──────┘

Because it is a more complex query but I tried to report the smallest example with the issue.

WA: toFloat64OrZero

SELECT if(startsWith('-1671.45', '.'), toFloat64OrZero(concat('0', '-1671.45')), toFloat64OrZero('-1671.45'))

Query id: 9994cec3-d8ce-4693-940b-c0de604a8675

┌─if(startsWith('-1671.45', '.'), toFloat64OrZero(concat('0', '-1671.45')), toFloat64OrZero('-1671.45'))─┐
│                                                                                               -1671.45 │
└────────────────────────────────────────────────────────────────────────────────────────────────────────┘

I agree, replacing toFloat64 by toFloat64OrZero in the original query fixes the issue :-)

Thank you!

@den-crane
Copy link
Contributor

constant folding

ah I see

SELECT
    materialize('-1671.45') AS x,
    if(startsWith(x, '.'), toFloat64(concat('0', x)), toFloat64(x))

Query id: 0979a0ad-83a5-49ff-af51-034efdf66317

┌─x────────┬─if(startsWith(materialize('-1671.45'), '.'), toFloat64(concat('0', materialize('-1671.45'))), toFloat64(materialize('-1671.45')))─┐
│ -1671.45 │                                                                                                                          -1671.45 │
└──────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

@alexey-milovidov
Copy link
Member

This is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants