Can you please comment your library so its actually usable

[kvtx]

It is almost impossible to use your go library because there is no documentation except the examples in your code base which is absolutely unacceptable. Comment your code.

[jkaflik]

@kvtx did you had a chance to take a look on https://clickhouse.com/docs/en/integrations/go ? It's linked directly in README. Is there anything missing you would like to include in the doc?

[kvtx]

My apologies for being a tad prickish on friday, i was frustrated by the following....

It is very unclear what the preferred methodology for query parameters is:

there is one example here: https://github.com/ClickHouse/clickhouse-go/blob/main/examples/clickhouse_api/query_parameters.go

another here:
https://clickhouse.com/docs/en/integrations/go#parameter-binding

The links in the github documentation take you to this

Now... given that was the first example ill walk you through what it was like to figure out how to use this properly from within Go.

In the screenshot below you can see the on hover for clickhouse query within my implemention:

this function accepts ...args any as its 3rd parameter, so you might assume that it actually takes a distribution of any (strings, ints etc) in all cases. However, it only takes any if you use the old "deprecated" bindings of ? or $ and not the qualified types bindings suggested for use by your docs/examples: {str:String}

In order to know that it doesn't actually take any but it instead takes a very specific driver.NamedValue type masked as any you either have to look directly at examples and make assumptions based on those examples, or you have to use the function without that correct input and get the error message, or trace the web of interfaces in the IDE until you find the real implementation where the error is thrown if the any cannot be parsed into the NamedValue.

The trace is not simple because of your interface pattern, trying to trace on query gets you here:

so instead, you must trace from the implementer of the interface which is the client... now this is a journey:

1. 

2. 

3. 

4. 

5. 

5 function traces later we finally have ONE single tiny comment that tells us how to use this thing:

	// validate if query contains a {<name>:<data type>} syntax, so it's intentional use of query parameters
	// parameter values will be loaded from `args ...any` for compatibility

this should be on the description of the interface method query at the very least.

AND we finally see that, without a comment or anything, this new NamedValue struct of string, any actually is, yet again, not actually allowed to be any but instead must actually be a string

Ok but better yet, it appears that this may actually be the deprecated method for implementing query parameters.

your documentations says this: query parameters (deprecated in favour of native query parameters)

but then gives no qualification for what a native query parameter is.

If you Google , the first thing that comes up is the reference example that appears to possible be the deprecated version.

but all of this is very unclear,

if you could just add a few comments in the code on the interface it would make integrating with your go library much more straight forward and not require cross referencing a bunch of docs. Of which, it is not particularly clear which are new and favored and which are old and out of favor.

All you would have to do to add go docs is copy paste into your go code in a comment... Im also making assumptions about which method is deprecated because i have not found anywhere that makes that fully clear except the if statement in the code that appears to short circuit on suggestion that we are preferring the new method.

Here is all you would need to at a minimum make this usable:

// Query accepts multiple bindvar options `?`, `@named`, `$`  and the deprecated `{param: type}` binding method.
/ /in order to use named parameters, you must pass the argument as type `NamedVar`
Query(ctx context.Context, query string, args ...any) (Rows, error)

//repeat for each of the follow `Select`, `Query Row`

In fact, if you can clear all this up for me I will make a PR and document the code for you!

[jkaflik]

@kvtx

Thanks for describing the problem you are facing in depth! I agree that the query parameters feature is specifically badly documented. I will follow up on this week.

Query parameters is a way to go: https://github.com/ClickHouse/clickhouse-go/blob/main/examples/clickhouse_api/query_parameters.go

Here is ClickHouse documentation on query parameters: https://clickhouse.com/docs/en/sql-reference/syntax#defining-and-using-query-parameters
You can also watch a recording explaining this: https://clickhouse.com/videos/how-to-use-query-parameters-in-clickhouse

Besides aligning on query parameters documentation, I think it's worth to invest in exported types comments. This library doesn't have it at all, but it's really worth to expose how is the logic handled with this! You can follow this issue.

[kvtx]

Much appreciated!

could you clarify which is the preferred Binding method?

[jkaflik]

@kvtx using ClickHouse query parameters

[kvtx]

thats these: {key:type}?

[jkaflik]

@kvtx yes: https://clickhouse.com/docs/en/sql-reference/syntax#defining-and-using-query-parameters

Feel free to reopen this issue if you have any further questions.