Comments (8)
@santaux I had more time to think about it, and I'm certain now that it's better to keep things as simple as possible in the adapter.
Positional parameters is a nice-to-have feature, but it should be supported by clickhouse itself.
Faking it in the adapter creates the unwanted discrepancy between db implementation, api of other clients (e.g. in other languages) and the current client's api.
from clickhouse_ecto.
@IvanIvanoff could you provide a code example for ecto query with multiple params with the same ?1
placeholder? I couldn't think of such cases in our code, that's why I though it will be safe to remove positional arguments. (like in mysql adapter)
from clickhouse_ecto.
@scarfacedeb For one of my use cases I'm writing plain sql and execute it with ClickhouseRepo.query
.
The example shows how to fill gaps when bucketing into time intervals:
query = """
SELECT SUM(value), time
FROM (
SELECT
toDateTime(intDiv(toUInt32(?4 + number * ?1), ?1) * ?1) as time,
toFloat64(0) AS value
FROM numbers(?2)
UNION ALL
SELECT toDateTime(intDiv(toUInt32(dt), ?1) * ?1) as time, sum(value) as value
FROM transfers
PREWHERE from IN (?3) AND NOT to IN (?3)
AND dt >= toDateTime(?4)
AND dt <= toDateTime(?5)
GROUP BY time
ORDER BY time
)
GROUP BY time
ORDER BY time
"""
args = [
interval,
span,
wallets,
from_datetime_unix,
to_datetime_unix
]
Without positional params I have to pass 11 arguments instead of 5.
Note that with the old behaviour you can achieve this by passing 5 params and fill the rest with nil
or any other value (due to the bug I'm fixing in my PR).
from clickhouse_ecto.
@IvanIvanoff I could see how positional params can be useful here.
But I think that clickhouse_ecto
is not the right place to implement them.
The actual substitution of ?
params with values is handled by clickhousex
in https://github.com/appodeal/clickhousex/blob/a643602777781187966d916278060d4e6073453a/lib/clickhousex/helpers.ex#L5
Therefore modifying bind_query_params
to handle more complex ?<number>
params would make them also work with clickhouse_ecto
(with small adjustments).
@santaux what do you think?
from clickhouse_ecto.
On second thought, clickhouse
itself doesn't support any placeholder params at all.
Therefore any params-related features will be implemented in the clickhousex
only, without the db support.
On the one hand, It gives certain flexibility to the implementation. On the other hand, it means extra support for that feature.
In contrast to postgres
that supports ordered params out of the box, and therefore it makes sense to use them in postgrex
.
I guess only the maintainer can tell if it's worth pursuing.
from clickhouse_ecto.
@scarfacedeb I'm still not convinced where it should be handled. clickhouse_ecto
translates the query from containing ?<num>
to plain query containing plain ?
and updates the argument list accordingly. In my opinion clickhousex
should implement only syntax that ClickHouse understands.
Are you aware of any such situation in other adapters and how they handle it?
Now when I think about it I'm also not sure if adding features not supported by ClickHouse brings more benefit or more harm.
Edit: Yeah, you managed to post the same thoughts just a second before me.
from clickhouse_ecto.
Now when I think about it I'm also not sure if adding features not supported by ClickHouse brings more benefit or more harm.
yep, that's exactly what I was thinking in the message above :)
Are you aware of any such situation in other adapters and how they handle it?
As far as my understanding goes,
postgrex supports positional params, because postgres itself supports them (see: https://github.com/elixir-ecto/postgrex/blob/43aa6b964043dd8726dba363245aebd3734bde47/lib/postgrex/messages.ex#L289)
Ecto.Adapters.Mysql
only supports ?
params. (and it looks like mariaex
sends those params directly: https://github.com/xerions/mariaex/blob/16d8abcfe479005651bd11050303382448664e6a/lib/mariaex/protocol.ex#L487)
from clickhouse_ecto.
what do you think?
I think that you were right when said that clickhouse itself doesn't support any placeholder params at all. And also, as @IvanIvanoff said:
Now when I think about it I'm also not sure if adding features not supported by ClickHouse brings more benefit or more harm.
🤔
from clickhouse_ecto.
Related Issues (12)
- Limit query does not work correctly
- Can not use ecto multy HOT 2
- ecto.migrate cannot create migration table HOT 1
- Hex.pm code/dependencies outdated HOT 1
- Ecto 3 HOT 1
- No migration rollbacks
- Doesn't work with the latest version of clickhousex
- Authorization doesn't work with later versions of Clickhouse HOT 1
- Possible typo HOT 2
- Ecto subqueries leads to missing select clause HOT 3
- WITH TOTALS dont work HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from clickhouse_ecto.