chore: handle case of empty child filter #214
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
============================================
+ Coverage 73.17% 73.26% +0.08%
- Complexity 1109 1115 +6
============================================
Files 96 96
Lines 4780 4784 +4
Branches 547 547
============================================
+ Hits 3498 3505 +7
+ Misses 1055 1054 -1
+ Partials 227 225 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...l/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java
Outdated
Show resolved
Hide resolved
| } | ||
| }] | ||
| }, { | ||
| "childFilter": [{}, { |
There was a problem hiding this comment.
Validation at the server seems fine, but wouldn't we also see if there is some issue on the client side? How is it forming an empty child filter?
There was a problem hiding this comment.
It was coming from QE tests. There is already an issue filed for them to fix. Still we can gracefully handle empty filter in query service
|
|
||
| private static final String QUERY_SERVICE_TEST_REQUESTS_DIR = "test-requests"; | ||
|
|
||
| protected TQueryServiceRequestType readQueryServiceRequest(String fileName) { |
There was a problem hiding this comment.
What is Tstands for? What about just having QueryServiceRequestType Is it how it was named in proto?
There was a problem hiding this comment.
I think it is to denote a generic class type. It is same as definer here by you - https://github.com/hypertrace/gateway-service/blob/1997536bcdddfc4d00aed3af7392fc1bb2ecdf91/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/AbstractServiceTest.java#L45
query-service-impl/src/test/java/org/hypertrace/core/query/service/AbstractServiceTest.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
No description provided.