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.

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

fix: Fix NPE while querying column having null value #160

Merged
merged 2 commits into from
Aug 5, 2022
Merged

Conversation

saxenakshitiz
Copy link
Contributor

This change fixes NPE while retrieving column having null value. The exception trace is as follows -

	at org.hypertrace.core.query.service.api.Value$Builder.setString(Value.java:1598) ~[query-service-api.jar:?]
	at org.hypertrace.core.query.service.postgres.PostgresBasedRequestHandler.convert(PostgresBasedRequestHandler.java:471) ~[query-service-impl.jar:?]
	at org.hypertrace.core.query.service.postgres.PostgresBasedRequestHandler.handleRequest(PostgresBasedRequestHandler.java:390) ~[query-service-impl.jar:?]
	at org.hypertrace.core.query.service.QueryServiceImpl.lambda$executeTransformedRequest$4(QueryServiceImpl.java:100) ~[query-service-impl.jar:?]
	at io.reactivex.rxjava3.internal.operators.mixed.MaybeFlatMapObservable$FlatMapObserver.onSuccess(MaybeFlatMapObservable.java:102) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.maybe.MaybeSwitchIfEmpty$SwitchIfEmptyMaybeObserver.onSuccess(MaybeSwitchIfEmpty.java:75) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.maybe.MaybeJust.subscribeActual(MaybeJust.java:36) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.core.Maybe.subscribe(Maybe.java:5375) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.maybe.MaybeSwitchIfEmpty.subscribeActual(MaybeSwitchIfEmpty.java:38) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.core.Maybe.subscribe(Maybe.java:5375) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.mixed.MaybeFlatMapObservable.subscribeActual(MaybeFlatMapObservable.java:49) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.core.Observable.subscribe(Observable.java:13173) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.observable.ObservableLift.subscribeActual(ObservableLift.java:58) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.core.Observable.subscribe(Observable.java:13173) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.mixed.SingleFlatMapObservable$FlatMapObserver.onSuccess(SingleFlatMapObservable.java:110) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.single.SingleFlatMap$SingleFlatMapCallback$FlatMapSingleObserver.onSuccess(SingleFlatMap.java:112) ~[rxjava-3.1.5.jar:?]
	at io.reactivex.rxjava3.internal.operators.single.SingleFlatMap$SingleFlatMapCallback$FlatMapSingleObserver.onSuccess(SingleFlatMap.java:112) ~[rxjava-3.1.5.jar:?]

@saxenakshitiz saxenakshitiz requested a review from a team August 5, 2022 11:48
@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #160 (76d86f8) into main (d9a094d) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #160      +/-   ##
============================================
+ Coverage     77.87%   77.91%   +0.03%     
  Complexity      758      758              
============================================
  Files            79       79              
  Lines          3300     3305       +5     
  Branches        368      369       +1     
============================================
+ Hits           2570     2575       +5     
  Misses          568      568              
  Partials        162      162              
Flag Coverage Δ
integration 77.91% <100.00%> (+0.03%) ⬆️
unit 70.29% <33.33%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ertrace/core/query/service/QueryServiceConfig.java 92.64% <100.00%> (+0.22%) ⬆️
.../service/postgres/PostgresBasedRequestHandler.java 39.66% <100.00%> (+0.75%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@github-actions

This comment has been minimized.

@@ -60,6 +60,8 @@ public class PostgresBasedRequestHandler implements RequestHandler {
private static final int DEFAULT_SLOW_QUERY_THRESHOLD_MS = 3000;
private static final Set<Operator> GTE_OPERATORS = Set.of(Operator.GE, Operator.GT, Operator.EQ);

private static final Value NULL_VALUE = Value.newBuilder().setString("null").build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: The null string is set in QS o/p in other handlers, so this fix will align postgres impl.

@github-actions

This comment has been minimized.

@saxenakshitiz saxenakshitiz merged commit a8fbb97 into main Aug 5, 2022
@saxenakshitiz saxenakshitiz deleted the fix_npe branch August 5, 2022 12:03
@github-actions
Copy link

github-actions bot commented Aug 5, 2022

Unit Test Results

  38 files  ±0    38 suites  ±0   9s ⏱️ -2s
246 tests +1  246 ✔️ +1  0 💤 ±0  0 ❌ ±0 

Results for commit a8fbb97. ± Comparison against base commit d9a094d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants