Skip to content

fix bug when FrontCodedIndexed has only the null value#13309

Merged
clintropolis merged 1 commit intoapache:masterfrom
clintropolis:front-coded-only-null
Nov 4, 2022
Merged

fix bug when FrontCodedIndexed has only the null value#13309
clintropolis merged 1 commit intoapache:masterfrom
clintropolis:front-coded-only-null

Conversation

@clintropolis
Copy link
Member

Description

This PR fixes a bug with FrontCodedIndexed when containing only a single null value. Because the null value is not actually stored in the "value buckets", it can result in a null pointer exception in FrontCodedIndexedWriter when serializing the column, and a buffer underflow in FrontCodedIndexed when creating an iterator on a FrontCodedIndexed with only a null value, since the values section is actually empty.

I also added bounds checking to the get method of FrontCodedIndexed as a safety measure, which if everything is written correctly shouldn't be needed, but also doesn't seem to add much performance impact so it seems worth the sanity checks it provides in the event

Benchmark              (query)  (rowsPerSegment)  (storageType)  (stringEncoding)  (vectorize)  Mode  Cnt      Score     Error  Units
SqlBenchmark.querySql       23           5000000           mmap              none        false  avgt    5  11256.550 ±  43.827  ms/op
SqlBenchmark.querySql       23           5000000           mmap              none        force  avgt    5  11255.502 ±  29.165  ms/op
SqlBenchmark.querySql       23           5000000           mmap     front-coded-4        false  avgt    5  11978.854 ±  27.574  ms/op
SqlBenchmark.querySql       23           5000000           mmap     front-coded-4        force  avgt    5  12724.125 ±  85.407  ms/op
SqlBenchmark.querySql       23           5000000           mmap    front-coded-16        false  avgt    5  13319.569 ±  41.002  ms/op
SqlBenchmark.querySql       23           5000000           mmap    front-coded-16        force  avgt    5  13300.229 ±  58.178  ms/op
SqlBenchmark.querySql       26           5000000           mmap              none        false  avgt    5     24.458 ±   0.771  ms/op
SqlBenchmark.querySql       26           5000000           mmap              none        force  avgt    5     20.168 ±   0.615  ms/op
SqlBenchmark.querySql       26           5000000           mmap     front-coded-4        false  avgt    5     24.781 ±   0.720  ms/op
SqlBenchmark.querySql       26           5000000           mmap     front-coded-4        force  avgt    5     20.343 ±   0.666  ms/op
SqlBenchmark.querySql       26           5000000           mmap    front-coded-16        false  avgt    5     24.626 ±   0.792  ms/op
SqlBenchmark.querySql       26           5000000           mmap    front-coded-16        force  avgt    5     20.578 ±   0.666  ms/op
SqlBenchmark.querySql       27           5000000           mmap              none        false  avgt    5    407.870 ±   4.176  ms/op
SqlBenchmark.querySql       27           5000000           mmap              none        force  avgt    5    245.788 ±   1.136  ms/op
SqlBenchmark.querySql       27           5000000           mmap     front-coded-4        false  avgt    5    416.001 ±   5.973  ms/op
SqlBenchmark.querySql       27           5000000           mmap     front-coded-4        force  avgt    5    248.944 ±   8.933  ms/op
SqlBenchmark.querySql       27           5000000           mmap    front-coded-16        false  avgt    5    419.480 ±   7.841  ms/op
SqlBenchmark.querySql       27           5000000           mmap    front-coded-16        force  avgt    5    256.627 ±   4.642  ms/op

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@clintropolis clintropolis changed the title fix bug when front-coded index has only the null value fix bug when FrontCodedIndexed has only the null value Nov 4, 2022
@clintropolis clintropolis merged commit d832919 into apache:master Nov 4, 2022
@clintropolis clintropolis deleted the front-coded-only-null branch November 4, 2022 12:26
@kfaraz kfaraz added this to the 25.0 milestone Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants