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

Unable to use ternary operator with named arguments #962

Closed
crisismaple opened this issue Apr 6, 2023 · 3 comments
Closed

Unable to use ternary operator with named arguments #962

crisismaple opened this issue Apr 6, 2023 · 3 comments
Labels

Comments

@crisismaple
Copy link
Contributor

Describe the bug

Bind function will return error clickhouse [bind]: mixed named, numeric or positional parameters when using ternary operator "?:" with named arguments.

Maybe we should skip the havePositional check when passing named arguments (since the regex expression may not cover all query scenarios using ternary operators or constraint "?").

clickhouse-go/bind.go

Lines 78 to 94 in b3c72d8

for _, v := range args {
switch v.(type) {
case driver.NamedValue, driver.NamedDateValue:
haveNamed = true
default:
}
if haveNamed && (haveNumeric || havePositional) {
return "", ErrBindMixedParamsFormats
}
}
if haveNamed {
return bindNamed(tz, query, args...)
}
if haveNumeric {
return bindNumeric(tz, query, args...)
}
return bindPositional(tz, query, args...)

Code example

func TestTernaryOperator(t *testing.T) {
	sqls := []string{
		`SELECT if(@arg1,@arg2,@arg3)`, // correct
		`SELECT @arg1?@arg2:@arg3`, // failed here
	}
	for _, sql := range sqls {
		_, err := bind(time.Local, sql,
			Named("arg1", 0),
			Named("arg2", 1),
			Named("arg3", 2))
		if err != nil {
			t.Fatal(err)
		}
	}
}
@crisismaple crisismaple added the bug label Apr 6, 2023
@jkaflik
Copy link
Contributor

jkaflik commented Apr 6, 2023

Hi, historically, I don't know why such a check was introduced. I will have to dig into it.

On a side note: did you consider using the native query parameters mechanism? #854

@crisismaple
Copy link
Contributor Author

This logic seems to be introduced long ago, since we are using an archived CH version (19.x) which not supporting directly pass parameters with query, we now have switched to use if function to replace all ternary operators as a work around.

I will also try to check binding strategies in other language implementations for clickhouse ;)

@jkaflik
Copy link
Contributor

jkaflik commented Apr 13, 2023

Fixed in #965

@jkaflik jkaflik closed this as completed Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants