feat(clickhouse sink): Add encoding.timestamp_format field

[Jeffail]

Allows users to specify a format for timestamp such that it can be parsed by Clickhouse as a DateTime data type.

By default we're not applying any formatting (sending as a string), this preserves backwards compatibility, it's also not an entirely unfavorable behavior since it has a higher precision. However, it's worth considering making unix formatting the default behavior here instead.

Closes #1443

[Jeffail]

Also for now I've implemented this as an enum in case we wish to add more formats later, but I'm also not convinced there's any reason not to use unix timestamp format so we could opt to change it to a boolean flag (called unix_timestamp?)

[Jeffail]

After having a moment to think about it I think the right course is to have a boolean flag format_unix_timestamp which when true results in the field timestamp being sent as a unix timestamp.

[binarylogic]

I think I agree. A few questions:

1. Would it not be better to treat all timestamps in the event this way? We have a timestamp type that any field can use.
2. This fits into our larger strategy of adding encoding options, which we discussed here. So I'm wondering if it makes more sense to add an encoding.timestamp_format option? This way we can add encoding.fields_whitelist and so on.
3. If unix is the preferred/optimal way to encode timestamp for Clickhouse then we should default to that instead of a string. I don't know enough about Clickhouse though.

[Jeffail]

If we can tie this into encoding options in my view that's the more "user empowering" way, it also makes the behavior clearer. Also yes on it applying to all timestamps.

Do we want to wait for the encoding work to be fully specced out, or add a field for this sink in the interim? If we add format_timestamp_unix (still cant figure a good name for this) then when encoding.timestamps_as_unix (or whatever) is added we can deprecate the old field.

[ghost]

I wonder how could we, maybe in subsequent PRs, also make it possible to either

• create two separate fields (one is DateTime and another one is UInt32 containing the number of nanoseconds after the last second);
• create a single Tuple, containing again a DateTime and a UInt32 with the number of nanoseconds.

These are two options frequently recommended to be used in ClickHouse for use cases where sub-second precision is needed. Also, some people might want to store, for example, milliseconds instead of nanoseconds.

[Hoverbear]

Is there anything holding up this PR?

[Jeffail]

We ought to expand the behavior so that all timestamp fields are encoded according to this setting, and not just the standard timestamp field. But other than that I think we can add this in without aggro.

[binarylogic]

I agree with @Jeffail. Can we go with an encoding.timestamp_format option? This will jive better with option 2 here. Do you agree @Hoverbear?

If possible, I'd like to get this in 0.8.0 for one of our important users.

[Jeffail]

Okay so for now I've:

1. Nested the field timestamp_format under encoding
2. Added an enum value rfc3339, which is the current behavior, I think it helps to make it explicit
3. Kept the format rfc3339 as default. For some users they will want proper timestamps and now they have the option. However, since it loses precision I don't think we should go as far as making it the default. Instead, we can revisit this (Ability to store sub-second precision in ClickHouse #1672) and add more encoding options that preserve precision later.

[binarylogic]

Perfect! Looks good to me.