Please help me understand why templating is needed here

Please help me understand why templating is needed here

If user uses % in the filter query then fmt.Sprintf was failing as % was treated as variable insertion operator. We could have escaped the characters but that could have more code changes. So used templates.

I do not think templating should be used because we need to make more code changes. What more changes are we talking about? There is one ClickHouseFormattedValue which should take of it.

I do not think templating should be used because we need to make more code changes. What more changes are we talking about? There is one ClickHouseFormattedValue which should take of it.

Apart from filter % can occur at multiple places, what if % is present in a key? So the cases keeps on increasing.
Using Template was safest bet for future changes or such scenarios.

Apart from filter % can occur at multiple places, what if % is present in a key? So the cases keeps on increasing

This is very unlikely. I don't think that justifies it. Assume such a key exists for the sake of argument. Does this fix solve all the issues? This is simply checking specific limit cases.

Apart from filter % can occur at multiple places, what if % is present in a key? So the cases keeps on increasing

This is very unlikely. I don't think that justifies it. Assume such a key exists for the sake of argument. Does this fix solve all the issues? This is simply checking specific limit cases.

In cases where we are using limit, it won't be a reproducible but template doesn't fix other such possible cases. We can use template everywhere to avoid the issues.

My point is that issue arose because of a special character, and it should be escaped. The idea of templatizing the whole query builder is a new effort from scratch; having seen the builder code, I think it will turn out to be a net negative and (most likely) will not fix the special character problems. I do not think we should introduce a templating here, and "mode code changes" is not a justifiable reason. I can think of many cases where not escaping is the source of bugs (try level = 'info in logs or something similar in traces or metrics). I do not want this PR to fix all potential such issues but escaping is the way to go, and it should start with this. I am open here others' thoughts on this.

// @nityanandagohain @rkssisodiya @ankitnayan

There must be many golang projects that deal with the same issue. It would be wise to look into a few of them and get inspiration. @makeavish please post your findings

Other projects use DB library query parameters methods instead of manual escaping. We should also use clickhouse named parameters. We are already using clickhouse named parameters for getFilteredSpan function in reader.go.

Closing this as we will go ahead with using clickhouse named operators: #3345