[common] Stop answering value predicates from a truncated BSI index - #9654
[common] Stop answering value predicates from a truncated BSI index#9654LuciferYang wants to merge 2 commits into
Conversation
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.
JingsongLi
left a comment
There was a problem hiding this comment.
This addresses a real false-negative pruning issue: sub-microsecond timestamp values can be omitted by the bitmap before residual filtering sees them. I reviewed the BSI mapper, RawFileSplitRead selection path and supported timestamp storage formats. On isolated base/head classpaths, the new case fails on the base and all seven index tests pass with this change; no blocking defect found in that scope.
Please add a table-level TIMESTAMP(9)+BSI regression for Parquet and ORC, comparing indexed/unindexed results for <> and strict range boundaries as well as null predicates. For the Java reader, use executeFilter(): REMAIN disables index pruning but does not itself provide exact residual filtering. I did not independently execute that table-level test, and Spark's usual microsecond timestamp type should not be used to claim coverage of this nanosecond case.
…n for parquet and orc
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 aBitmapIndexResultis not one:RawFileSplitReadhands the bitmap toApplyBitmapIndexRecordReader, which reads exactly those rows. Sots <> '...000000000'drops a row whose nanoseconds differ from the literal, andts = '...'selects it.This fixes it on the read side. When the value mapper truncates for the column's type,
createReaderwraps the reader so that onlyIS NULLandIS NOT NULLcome from the index and every value predicate falls through toFileIndexReader'sREMAINdefault, 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> 3would 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 viatoSQLTimestamp(), andOrcTimestampColumnVectorreadsvector.nanos[i] % 1_000_000without 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
BitSliceIndexBitmapFileIndexTestgets two cases.testSubMicrosecondTimestampIndexAnswersNoValuePredicatebuilds 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 returnREMAIN, and thatIS NULLandIS NOT NULLstill return exact bitmaps.testMicrosecondTimestampIndexStillAnswersValuePredicatespins the other side of the boundary: at precision 6 the index still answers value predicates with exact bitmaps.Verified both directions. With
BitSliceIndexBitmapFileIndex.javareverted to master, the first assertion fails withexpected: FileIndexResult$1 but was: BitmapIndexResult; with the change in place the class passes 7/7.