-
Notifications
You must be signed in to change notification settings - Fork 6
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
Computing document _id
on the fly
#133
Conversation
_id
on the fly_id
on the fly
if v, ok := cw.Table.Cols[timestampColumnName]; !ok { | ||
switch v.Type.String() { | ||
case clickhouse.DateTime64.String(): | ||
statement = fmt.Sprintf("toUnixTimestamp64Milli(%s) IN (%s) ", strconv.Quote(timestampColumnName), ids) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to double-check and they indeed called the function *Milli
and not *Millis
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have two worries:
- Dashboard UI might rely on ids being unique for queries.
- We might want to have some format that we internally could recognize. E.g. t[timestamp hex without 0x]h(irrelevant counter).
Though fine to merge as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We found one regression it introduce while traversing docs, Przemek is working to fix this.
We found one regression it introduce while traversing docs, Przemek is working to fix this.
What's the scenario? |
Open the log view, click on one document and click the next arrow on the document. If ids clash, it does not work. It also highlights all documents instead of the current one. |
_id
on the fly_id
on the fly
We're adding additional field to the index configuration (
timestampField
), which needs to be DateTime/DateTime64 at the ClickHouse end.It's not required when you don't want to generate document IDs, but in case you do, that's the way to go.