Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix NPE in registeredLookup extractionFn when "optimize" is not provided. #3064

Merged
merged 1 commit into from Jun 3, 2016
Merged

Fix NPE in registeredLookup extractionFn when "optimize" is not provided. #3064

merged 1 commit into from Jun 3, 2016

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Jun 3, 2016

No description provided.

@gianm gianm added the Bug label Jun 3, 2016
@gianm gianm added this to the 0.9.1 milestone Jun 3, 2016
drcrallen
drcrallen reviewed Jun 3, 2016
View reviewed changes
processing/src/main/java/io/druid/query/lookup/RegisteredLookupExtractionFn.java
@@ -55,7 +55,7 @@ public RegisteredLookupExtractionFn(
this.replaceMissingValueWith = replaceMissingValueWith;
this.retainMissingValue = retainMissingValue;
this.injective = injective;
this.optimize = optimize;
this.optimize = optimize == null ? true : optimize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optimize is an experimental path, the input variable should be boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just mimicking the behavior in LookupDimensionSpec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take no stand on what the default should be, other than that they should behave the same way. Happy to change them both to default false.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arg... consistency for now I agree is better.

@gianm
Copy link
Contributor Author

gianm commented Jun 3, 2016

travis fail due to #3067

@gianm gianm closed this Jun 3, 2016
@gianm gianm reopened this Jun 3, 2016
@drcrallen
Copy link
Contributor

👍

@fjy fjy closed this Jun 3, 2016
@fjy fjy reopened this Jun 3, 2016
@nishantmonu51
Copy link
Member

👍, after travis

@gianm
Copy link
Contributor Author

gianm commented Jun 3, 2016

https://travis-ci.org/druid-io/druid/jobs/134941003

Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 60.125 sec <<< FAILURE! - in io.druid.server.QueryResourceTest
testSecuredGetServer(io.druid.server.QueryResourceTest)  Time elapsed: 60.022 sec  <<< ERROR!
java.lang.Exception: test timed out after 60000 milliseconds
    at sun.misc.Unsafe.park(Native Method)
    at java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:836)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:997)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1304)
    at java.util.concurrent.CountDownLatch.await(CountDownLatch.java:231)
    at io.druid.server.QueryResourceTest.testSecuredGetServer(QueryResourceTest.java:323)

@gianm gianm closed this Jun 3, 2016
@gianm gianm reopened this Jun 3, 2016
@fjy fjy closed this Jun 3, 2016
@fjy fjy reopened this Jun 3, 2016
@gianm
Copy link
Contributor Author

gianm commented Jun 3, 2016

https://travis-ci.org/druid-io/druid/builds/135057242 (#2253)

Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 66.962 sec <<< FAILURE! - in io.druid.curator.announcement.AnnouncerTest
testSanity(io.druid.curator.announcement.AnnouncerTest)  Time elapsed: 61.106 sec  <<< ERROR!
java.lang.Exception: test timed out after 60000 milliseconds
    at sun.misc.Unsafe.park(Native Method)
    at java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:226)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedNanos(AbstractQueuedSynchronizer.java:1033)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.tryAcquireSharedNanos(AbstractQueuedSynchronizer.java:1326)
    at java.util.concurrent.CountDownLatch.await(CountDownLatch.java:282)
    at org.apache.curator.test.Timing.awaitLatch(Timing.java:120)
    at io.druid.curator.announcement.AnnouncerTest.testSanity(AnnouncerTest.java:101)

@gianm gianm closed this Jun 3, 2016
@gianm gianm reopened this Jun 3, 2016
@b-slim b-slim merged commit 54139c6 into apache:master Jun 3, 2016
@gianm gianm mentioned this pull request Jun 3, 2016
Merged
@gianm gianm modified the milestones: 0.9.2, 0.9.1 Sep 23, 2016
seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Feb 25, 2022
@navis
apache#3064 Fix NPE in sketch query for null metric value
ab0acf2
@gianm gianm deleted the rlef-npe branch September 23, 2022 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
0.9.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gianm @drcrallen @nishantmonu51 @fjy @b-slim