Skip to content

[common] Stop answering value predicates from a truncated BSI index - #9654

Open
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/bsi-subsecond-timestamp-unusable
Open

[common] Stop answering value predicates from a truncated BSI index#9654
LuciferYang wants to merge 1 commit into
apache:masterfrom
LuciferYang:fix/bsi-subsecond-timestamp-unusable

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

Purpose

close #9645

A BSI index maps TIMESTAMP through Timestamp#toMicros(), so on a column with precision above 6 two values that differ only below a microsecond share one indexed value. That would be harmless for a candidate set, but a BitmapIndexResult is not one: RawFileSplitRead hands the bitmap to ApplyBitmapIndexRecordReader, which reads exactly those rows. So ts <> '...000000000' drops a row whose nanoseconds differ from the literal, and ts = '...' selects it.

This fixes it on the read side. When the value mapper truncates for the column's type, createReader wraps the reader so that only IS NULL and IS NOT NULL come from the index and every value predicate falls through to FileIndexReader's REMAIN default, which leaves the file to be read and filtered normally. Null-ness does not depend on the truncated digits, so those two questions still prune. The index format and the writer are untouched, which also means index files already written are read correctly from now on.

The alternative was to store nanos and bump the format version. I did not take it: it breaks old readers for every type that uses BSI, and nanoseconds since the epoch overflow int64 in 2262 while microseconds reach year 294247.

The condition is precision > 6, matching what the declared type can represent rather than what the mapper keeps. A column declared TIMESTAMP(4..6) cannot hold sub-microsecond values in the first place, so an index that stores microseconds is faithful to it, and widening the condition to > 3 would cost every correct TIMESTAMP(4..6) table its BSI pruning for nothing.

One related thing I found while checking that boundary, and deliberately left alone here: nothing on the write path normalizes a value to its column's declared precision, and ORC round-trips whatever nanoseconds it was given. FieldWriterFactory.visit(TimestampType) writes full nanos via toSQLTimestamp(), and OrcTimestampColumnVector reads vector.nanos[i] % 1_000_000 without consulting the precision. Parquet does clamp (MILLIS at precision 3 and below, MICROS at 4 to 6), so on Parquet a precision-6 column can never read back sub-microsecond digits. That gap belongs to the write path or to the ORC reader, not to the file index, so it is out of scope for this PR.

Tests

BitSliceIndexBitmapFileIndexTest gets two cases. testSubMicrosecondTimestampIndexAnswersNoValuePredicate builds a TIMESTAMP(9) index over two values half a microsecond apart plus a null, asserts that equal, not-equal, in, not-in, less-than, greater-than and between all return REMAIN, and that IS NULL and IS NOT NULL still return exact bitmaps. testMicrosecondTimestampIndexStillAnswersValuePredicates pins the other side of the boundary: at precision 6 the index still answers value predicates with exact bitmaps.

Verified both directions. With BitSliceIndexBitmapFileIndex.java reverted to master, the first assertion fails with expected: FileIndexResult$1 but was: BitmapIndexResult; with the change in place the class passes 7/7.

getTimeStampMapper stores micros for any TIMESTAMP precision above 3, so
for precision 7-9 two values that differ below a microsecond share one
indexed value. The BSI bitmap is the row set RawFileSplitRead reads, so
ts <> '...000000000' dropped the row whose nanoseconds differed and
ts = '...' selected it: wrong results, not coarse pruning.

Answer REMAIN for every value predicate on such a column, which leaves
the rows to be read and filtered. Null-ness does not depend on the
truncated digits, so IS NULL and IS NOT NULL still come from the index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] BSI index truncates TIMESTAMP(7-9) to micros and drops rows from the result

1 participant