fix(security): Upgrade druid version to 35.0.1#26820
Merged
nishithakbhaskaran merged 1 commit intoprestodb:masterfrom Feb 18, 2026
Merged
fix(security): Upgrade druid version to 35.0.1#26820nishithakbhaskaran merged 1 commit intoprestodb:masterfrom
nishithakbhaskaran merged 1 commit intoprestodb:masterfrom
Conversation
Contributor
Reviewer's GuideUpgrades the Druid dependency to 35.0.1 and introduces a custom PrestoQueryableIndex implementation plus API-adjustment shims to align with the newer Druid segment APIs, while explicitly pinning lz4-java and Rhino versions to remediate security vulnerabilities. Sequence diagram for V9SegmentIndexSource.loadIndex using PrestoQueryableIndexsequenceDiagram
participant Caller
participant V9SegmentIndexSource
participant SegmentColumnSource
participant GenericIndexed
participant ColumnDescriptor
participant PrestoQueryableIndex
Caller->>V9SegmentIndexSource: loadIndex(columnHandles)
V9SegmentIndexSource->>SegmentColumnSource: getColumnData(INDEX_METADATA_FILE_NAME)
SegmentColumnSource-->>V9SegmentIndexSource: byte[] indexData
V9SegmentIndexSource->>V9SegmentIndexSource: wrap in ByteBuffer indexBuffer
V9SegmentIndexSource->>GenericIndexed: read(indexBuffer, STRING_STRATEGY, null)
GenericIndexed-->>V9SegmentIndexSource: GenericIndexed dummyRead
V9SegmentIndexSource->>GenericIndexed: read(indexBuffer, STRING_STRATEGY, null)
GenericIndexed-->>V9SegmentIndexSource: GenericIndexed allDimensions
V9SegmentIndexSource->>V9SegmentIndexSource: parse Interval dataInterval
V9SegmentIndexSource->>V9SegmentIndexSource: build availableDimensions list
loop each columnName
V9SegmentIndexSource->>SegmentColumnSource: getColumnData(columnName)
SegmentColumnSource-->>V9SegmentIndexSource: byte[] columnData
V9SegmentIndexSource->>V9SegmentIndexSource: wrap in ByteBuffer columnBuffer
V9SegmentIndexSource->>ColumnDescriptor: read(columnBuffer, ColumnConfig.DEFAULT, null, null)
ColumnDescriptor-->>V9SegmentIndexSource: ColumnHolder columnHolder
V9SegmentIndexSource->>V9SegmentIndexSource: put Supplier in columns map
end
V9SegmentIndexSource->>V9SegmentIndexSource: build Indexed availableDimensions
V9SegmentIndexSource->>PrestoQueryableIndex: new PrestoQueryableIndex(dataInterval, availableDimensions, columns, metadata, segmentBitmapSerdeFactory.getBitmapFactory())
PrestoQueryableIndex-->>V9SegmentIndexSource: QueryableIndex instance
V9SegmentIndexSource-->>Caller: QueryableIndex
Class diagram for new PrestoQueryableIndex and its usage in V9SegmentIndexSourceclassDiagram
direction LR
class V9SegmentIndexSource {
-SegmentColumnSource segmentColumnSource
-SegmentBitmapSerdeFactory segmentBitmapSerdeFactory
+V9SegmentIndexSource(SegmentColumnSource segmentColumnSource)
+QueryableIndex loadIndex(List columnHandles) IOException
-ColumnDescriptor readColumnDescriptor(ByteBuffer byteBuffer) IOException
-ColumnHolder createColumnHolder(String columnName)
}
class PrestoQueryableIndex {
-Interval dataInterval
-Indexed availableDimensions
-Map~String, Supplier~ColumnHolder~~ columns
-Metadata metadata
-BitmapFactory bitmapFactory
+PrestoQueryableIndex(Interval dataInterval, Indexed availableDimensions, Map~String, Supplier~ColumnHolder~~ columns, Metadata metadata, BitmapFactory bitmapFactory)
+Interval getDataInterval()
+int getNumRows()
+Indexed getAvailableDimensions()
+List~String~ getColumnNames()
+BaseColumnHolder getColumnHolder(String columnName)
+Metadata getMetadata()
+List~OrderBy~ getOrdering()
+Map~String, DimensionHandler~ getDimensionHandlers()
+BitmapFactory getBitmapFactoryForDimensions()
+void close()
}
class QueryableIndex {
<<interface>>
+Interval getDataInterval()
+int getNumRows()
+Indexed getAvailableDimensions()
+List~String~ getColumnNames()
+BaseColumnHolder getColumnHolder(String columnName)
+Metadata getMetadata()
+List~OrderBy~ getOrdering()
+Map~String, DimensionHandler~ getDimensionHandlers()
+BitmapFactory getBitmapFactoryForDimensions()
+void close()
}
class ColumnDescriptor {
+ColumnHolder read(ByteBuffer byteBuffer, ColumnConfig columnConfig, Object arg1, Object arg2) IOException
}
class GenericIndexed {
+static GenericIndexed read(ByteBuffer buffer, Object strategy, Object arg)
}
class SegmentColumnSource {
+byte[] getColumnData(String columnName)
}
class SegmentBitmapSerdeFactory {
+BitmapFactory getBitmapFactory()
}
V9SegmentIndexSource --> SegmentColumnSource : uses
V9SegmentIndexSource --> SegmentBitmapSerdeFactory : uses
V9SegmentIndexSource --> PrestoQueryableIndex : creates
PrestoQueryableIndex ..|> QueryableIndex
V9SegmentIndexSource --> ColumnDescriptor : reads
V9SegmentIndexSource --> GenericIndexed : uses static read
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
ShahimSharafudeen
force-pushed
the
lz4-java_cve_fix_druid
branch
2 times, most recently
from
a3b3aad to
3cf9e4b
Compare
prestodb-ci
requested review from
a team,
bibith4 and
jkhaliqi
and removed request for
a team
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
PrestoQueryableIndex,getNumRows()currently returns 0 andgetOrdering()/getDimensionHandlers()return empty collections; if this index is used by any code paths that rely on row counts, ordering, or dimension handlers, consider wiring these up to the underlying column data (or adding a short comment explaining why these can safely be empty/zero in this context). - In
PrestoQueryableIndex.getColumnHolder, you are castingColumnHoldertoBaseColumnHolder; if some implementations are notBaseColumnHolder, this will fail at runtime, so consider returningColumnHolderdirectly (if the interface allows) or adding a type check with a clearer failure mode.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PrestoQueryableIndex`, `getNumRows()` currently returns 0 and `getOrdering()` / `getDimensionHandlers()` return empty collections; if this index is used by any code paths that rely on row counts, ordering, or dimension handlers, consider wiring these up to the underlying column data (or adding a short comment explaining why these can safely be empty/zero in this context).
- In `PrestoQueryableIndex.getColumnHolder`, you are casting `ColumnHolder` to `BaseColumnHolder`; if some implementations are not `BaseColumnHolder`, this will fail at runtime, so consider returning `ColumnHolder` directly (if the interface allows) or adding a type check with a clearer failure mode.
## Individual Comments
### Comment 1
<location> `presto-druid/src/main/java/com/facebook/presto/druid/segment/PrestoQueryableIndex.java:58-65` </location>
<code_context>
+ this.bitmapFactory = bitmapFactory;
+ }
+
+ @Override
+ public Interval getDataInterval()
+ {
+ return dataInterval;
+ }
+
+ @Override
+ public int getNumRows()
+ {
+ return 0;
</code_context>
<issue_to_address>
**issue (bug_risk):** getNumRows() returning 0 is likely incorrect and can break consumers relying on row count.
Always returning 0 will cause any cardinality-based logic (planning, filters, sanity checks) to treat this segment as empty. If the actual row count is known, pass it through instead of hardcoding. If it truly isn’t available, make that explicit (e.g., documented limitation) and consider returning a reasonable estimate rather than 0.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
presto-druid/src/main/java/com/facebook/presto/druid/segment/PrestoQueryableIndex.java
Show resolved
Hide resolved
| Indexed<String> indexed = new ListIndexed<>(availableDimensions); | ||
| // TODO: get rid of the time column by creating Presto's SimpleQueryableIndex impl | ||
| return new SimpleQueryableIndex( | ||
| return new PrestoQueryableIndex( |
Contributor
Author
There was a problem hiding this comment.
In Druid 35.0.1, the SimpleQueryableIndex class was changed from a concrete class to an abstract class. This introduced a breaking API change that prevents direct instantiation using the new keyword. Therefore, I created PrestoQueryableIndex, a custom concrete implementation of the QueryableIndex interface.
bibith4
reviewed
Jan 1, 2026
| public V9SegmentIndexSource(SegmentColumnSource segmentColumnSource) | ||
| { | ||
| this.segmentColumnSource = requireNonNull(segmentColumnSource, "segmentColumnSource is null"); | ||
| NullHandling.initializeForTests(); |
Contributor
There was a problem hiding this comment.
@ShahimSharafudeen Could you please clarify why this line was removed, since it prevents the IllegalStateException that occurs when the code checks null-handling logic?
Contributor
Author
There was a problem hiding this comment.
In Apache Druid 35.0.1, the old null-handling legacy behavior and associated configs/constants (often referred to as NullHandling, druid.generic.useDefaultValueForNull, etc.) have been deprecated and removed because the project has fully moved to ANSI SQL–compliant null semantics, and the legacy behavior is no longer supported.
ShahimSharafudeen
force-pushed
the
lz4-java_cve_fix_druid
branch
from
3cf9e4b to
19e657a
Compare
Contributor
|
@ShahimSharafudeen imported this issue as lakehouse/presto #26820 |
pratyakshsharma
approved these changes
Jan 5, 2026
Contributor
pratyakshsharma
left a comment
pratyakshsharma
left a comment
There was a problem hiding this comment.
Thank you for making the changes as discussed.
bibith4
approved these changes
Jan 6, 2026
Contributor
|
Thanks for the release note! Some formatting nits: |
Contributor
Author
Thank you, @steveburnett, for the review. I have updated the release notes according to your feedback. |
Contributor
Thank you @ShahimSharafudeen! Having good release notes in the PR helps us at the end of the release cycle, by reducing the work needed for the PR to add the release notes to the documentation (example #26726 ). Because the release notes documentation PR is a blocker for the Presto release cycle, reducing the work needed to merge that doc PR helps us get the new Presto release out faster. |
Contributor
Author
|
@tdcmeehan — Could you please review this at your convenience? |
jkhaliqi
approved these changes
Jan 13, 2026
7 tasks
ShahimSharafudeen
force-pushed
the
lz4-java_cve_fix_druid
branch
from
19e657a to
127dcf7
Compare
tdcmeehan
approved these changes
Feb 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Upgrade druid to version 35.0.1 to address CVE-2024-53990 and CVE-2025-12183
Since the CVE fixes for lz4-java and Rhino are not included in Druid 35.0.1, we resolved the vulnerabilities by upgrading the corresponding transitive dependencies.
Upgrade lz4-java to version 1.10.2 to address CVE-2025-66566
Upgrade rhino to version 1.8.1 to address CVE-2025-66453
Motivation and Context
Impact
Test Plan
Tested in Local :
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.