Fixing the regression caused by #1172#1381
Merged
Merged
Conversation
Member
|
👍 |
Member
|
this failure was undetected from a long time, |
Member
|
@himanshug Happy to merge once you squash commits. Tests would be nice indeed |
Contributor
Author
|
Sure, I'm going to add the UT pretty soon. However, I wanted to keep the commit history as it has the story of what happened. If I squash them then the information of how really eagerness is applied is lost. currently, one could go to the specific commit (himanshug@5473f7a) to see what really changed overall. Please let me know if that does not sound reasonable to you. |
Member
|
@himanshug sounds reasonable. In that case let's update the second commit message to indicates what this fixes, so it doesn't look like we undo and redo the same thing. |
which caused following regression
it fails when we issue the SegmentMetadataQuery by setting {"bySegment" : true} in context with exception -
java.lang.ClassCastException: io.druid.query.Result cannot be cast to io.druid.query.metadata.metadata.SegmentAnalysis
at io.druid.query.metadata.SegmentMetadataQueryQueryToolChest$4.compare(SegmentMetadataQueryQueryToolChest.java:222) ~[druid-processing-0.7.3-SNAPSHOT.jar:0.7.3-SNAPSHOT]
at com.google.common.collect.NullsFirstOrdering.compare(NullsFirstOrdering.java:44) ~[guava-16.0.1.jar:?]
at com.metamx.common.guava.MergeIterator$1.compare(MergeIterator.java:46) ~[java-util-0.27.0.jar:?]
at com.metamx.common.guava.MergeIterator$1.compare(MergeIterator.java:42) ~[java-util-0.27.0.jar:?]
at java.util.PriorityQueue.siftUpUsingComparator(PriorityQueue.java:649) ~[?:1.7.0_80]
… regression caused by commit 55ebf0c it fails when we issue the SegmentMetadataQuery by setting {"bySegment" : true} in context with exception - java.lang.ClassCastException: io.druid.query.Result cannot be cast to io.druid.query.metadata.metadata.SegmentAnalysis at io.druid.query.metadata.SegmentMetadataQueryQueryToolChest$4.compare(SegmentMetadataQueryQueryToolChest.java:222) ~[druid-processing-0.7.3-SNAPSHOT.jar:0.7.3-SNAPSHOT] at com.google.common.collect.NullsFirstOrdering.compare(NullsFirstOrdering.java:44) ~[guava-16.0.1.jar:?] at com.metamx.common.guava.MergeIterator$1.compare(MergeIterator.java:46) ~[java-util-0.27.0.jar:?] at com.metamx.common.guava.MergeIterator$1.compare(MergeIterator.java:42) ~[java-util-0.27.0.jar:?] at java.util.PriorityQueue.siftUpUsingComparator(PriorityQueue.java:649) ~[?:1.7.0_80]
…ssing executor by converting the Sequence into List
himanshug
force-pushed
the
fix_seg_metadata_regression
branch
from
5473f7a to
723df73
Compare
Contributor
Author
|
@xvrl @nishantmonu51 added the UT |
Member
|
@himanshug great, once build passes I'll merge, and then we can cherry-pick into 0.7.x branch to hopefully release 0.7.3 as stable :) |
xvrl
added a commit
that referenced
this pull request
May 22, 2015
Fixing the regression caused by #1172
This was referenced May 26, 2015
FrankChen021
pushed a commit
that referenced
this pull request
May 29, 2026
* Bump jackson to 2.21.3 Jackson 2.21 (issue #1381) changed the default resolution of @JacksonInject when combined with @JsonProperty on the same parameter: the injected value now wins over the JSON value, where 2.20 treated the inject as a fallback used only when JSON did not supply one. DruidNode's serviceName, port, and tlsPort parameters carry both annotations, with JSON expected to win when supplied — this is how DruidNode JSON config files have always worked. Add the explicit useInput = OptBoolean.TRUE to restore that contract. A repo-wide audit confirmed DruidNode's three parameters are the only sites in Druid where @JacksonInject and @JsonProperty annotate the same parameter; everywhere else the annotations are on distinct parameters and are unaffected. Also adds the previously-missing license entry for org.jspecify:jspecify 1.0.0 in extensions-core/kubernetes-extensions, which the check-licenses dependency report flagged. * Preserve @JacksonInject metadata in GuiceAnnotationIntrospector findInjectableValue was returning JacksonInject.Value.forId(id), which strips useInput and optional from the original annotation. Production deserialization happens to remain correct under jackson 2.21 because AnnotationIntrospectorPair.findInjectableValue falls back to the secondary (default Jackson) introspector and merges the recovered useInput onto the primary's Value via withUseInput. That fallback is undocumented as part of the introspector contract and would silently regress if the pair semantics change, or if this introspector were ever installed standalone for a special-purpose mapper. Construct the Value via JacksonInject.Value.from(annotation) .withId(id) so the introspector returns a complete Value on its own and no longer relies on the pair to fix it up. The annotation lookup is hoisted to the top of findInjectableValue so the non-null contract between it and findGuiceInjectId is explicit — findGuiceInjectId now documents the precondition and trusts the caller to verify, eliminating the duplicate getAnnotation call. Defensive cleanup motivated by FasterXML/jackson-databind#1381; no observable behavior change.
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
3 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.
Fixing the regression caused by #1172
Reverted the use of ChainedExecutionQueryRunner and used simple approach to enforce eagerness.