Support transform functions with AVG aggregation function #4557
Conversation
|
@Jackie-Jiang , the reason for creating a separate test file is because InnerSegment/InterSegment queries test extend BaseSingleValueQueriesTest and use the entire avro dataset to build test segments. It is even difficult to predict if the queries are returning correct results since we don't know what data is there in avro files. So I created a separate test that just extends BaseQueriesTest to get helper methods and created 2 segments with small data to do both inner and inter segment tests and verify correctness |
|
|
||
| @BeforeClass | ||
| public void setup() { | ||
| _schema = new Schema(); |
There was a problem hiding this comment.
Use Schema.Builder to build the schema
| private List<SegmentDataManager> _segmentDataManagers; | ||
|
|
||
| @BeforeClass | ||
| public void setup() { |
There was a problem hiding this comment.
(nit) We usually use setUp()
| } | ||
|
|
||
| @AfterClass | ||
| public void destroy() { |
There was a problem hiding this comment.
(nit) We usually use tearDown()
| try { | ||
| final List<GenericRow> rows = createDataSet(10); | ||
|
|
||
| final RecordReader recordReader = new GenericRowRecordReader(rows, _schema); |
There was a problem hiding this comment.
Resource leak, use try-with resource
There was a problem hiding this comment.
Changed to try with resource but in this case we were not leaking resource as record reader.close() is a NO-OP
| final List<GenericRow> segmentTwoRows = createDataSet(10); | ||
|
|
||
| final RecordReader recordReaderOne = new GenericRowRecordReader(segmentOneRows, _schema); | ||
| final RecordReader recordReaderTwo = new GenericRowRecordReader(segmentTwoRows, _schema); |
There was a problem hiding this comment.
Resource leak, use try-with resource
There was a problem hiding this comment.
Changed to try with resource but in this case we were not leaking resource as record reader.close() is a NO-OP
| } | ||
|
|
||
| private void createSegment( | ||
| Schema schema, |
There was a problem hiding this comment.
Please reformat the file with the Pinot Style
| FileUtils.deleteQuietly(INDEX_DIR); | ||
| } | ||
|
|
||
| private void delete() { |
There was a problem hiding this comment.
Rename to destroySegments() for clarity
Codecov Report
@@ Coverage Diff @@
## master #4557 +/- ##
============================================
+ Coverage 56.15% 64.57% +8.42%
Complexity 20 20
============================================
Files 1064 1064
Lines 55318 55319 +1
Branches 8063 8063
============================================
+ Hits 31062 35722 +4660
+ Misses 21778 16967 -4811
- Partials 2478 2630 +152
Continue to review full report at Codecov.
|
|
Addressed the comments |
* Support transform functions with AVG aggregation function * address comments
Support queries like:
SELECT AVG(SUB(col1, col2) FROM foo
SELECT AVG(SUB(DIV(col1, col2), DIV(col3, col4))) FROM foo
They were failing in AvgAggregationFunction as TransformBlockValSet() did not implement getValueType() method
Also added operand checks for transform functions for acceptable data types
Testing:
Added TransformQueriesTest to test both inner-segment and inter-segment, inter-server queries with custom data