Improvement in NULL Handling for Datetime columns

[KKcorps]

if allowNullTimeValue is set to true, pinot fills in the System.currentMillis timestamp in place of nulls. This happens in the following piece of code in NullValueTransformer class

    // handle null value in time column
    if (_defaultTimeValueFormat != null && record.getValue(_dateTimeFieldSpec.getName()) == null) {
      String timeValueStr = _defaultTimeValueFormat.fromMillisToFormat(System.currentTimeMillis());
      Object timeValue = _dateTimeFieldSpec.getDataType().convert(timeValueStr);
      record.putDefaultNullValue(_dateTimeFieldSpec.getName(), timeValue);
    }

This default value however creates an unpredictable behaviour from querying perspective. e.g. I inserted data for duration from year 2016 to 2018 but suddenly I find there is a timestamp for year 2022 in my data.

Another scenario is where I re-insert my data after clearing the table and find totally different values.

Pinot either needs to set this value to Epoch 0 or move on to some better default.

[kishoreg]

How about supporting NULL by default for time column?

[xiangfu0]

@Jackie-Jiang
Maybe we can make new data types supporting null value by default?

Also for TIMESTAMP, I feel both deterministic value or current timestamp can be default null value.

[KKcorps]

@kishoreg one of the drawback of Pinot's current NULL support is that it operates via BitMap index and is not reflected anywhere in the data. This will again affect when the data is finally viewed by the user.

[Jackie-Jiang]

How about allowing users to customize the default time value, and fill the current time when no customized default time value is provided?

[KKcorps]

That also works. But our main objective for long term should be to enable native NULL handling in pinot so that we should not need to fill it with default values in such cases. I think the blocker to do that currently would be indexing when there are nulls.

[Jackie-Jiang]

That are 2 orthogonal problems to solve. We need to decide a default value for the time column, but can also enable the null value support (which is segment level) by default. When null value support is enabled (currently we need to explicitly enable it by setting nullHandlingEnabled to true in tableIndexConfig), we can skip the null time columns by using timeCol IS NOT NULL. In the future we can potentially add a config to automatically rewrite the query to skip the null values.
@KKcorps Do you think enabling the null handling and add a filter can solve your problem?

[Jackie-Jiang]

#8310

[jadami10]

two followups here:

• can we update the docs now that allowNullTimeValue is removed
• why did we limit user-specified defaults to 1971 when 1970 is start of epoch? our current system is using 0 as the default

[jadami10]

I'm finding it a bit harder to track down, but what does the offline spark job do with null times?

[Jackie-Jiang]

@jadami10 Good catch on the stale doc, updated.
We perform the time column value validation (must be in the range of 1971-2071) to prevent user configuration error of mismatching value and time unit. (E.g. if the time value is in millisSinceEpoch but user specify the time unit as SECONDS, then the segment will fail the validation). For the main time column, we probably don't want to use 0 as the default value because for hybrid table setup, the record will be ignored because of the time boundary.
The offline spark job should follow the same behavior here