maytasm commented on a change in pull request #12185:
URL: https://github.com/apache/druid/pull/12185#discussion_r840927933
##########
File path:
processing/src/test/java/org/apache/druid/segment/data/IncrementalIndexTest.java
##########
@@ -725,4 +728,171 @@ public void testDynamicSchemaRollup() throws
IndexSizeExceededException
Assert.assertEquals(2, index.size());
}
+
Review comment:
- Mix of aggregators with incompatible type. (i.e. compaction with
LongSum but some segment has existing metric as String and some has number) is
a sad path and will result in the ingestion task failing. The error message
will mentioned that this is because isPreserveExistingMetrics is enabled
(isPreserveExistingMetrics can be disable to fallback to original behavior).
Added this as a test case.
- If non-rolled up has columns with the same name as aggregators in rolled
up and isPreserveExistingMetrics is enabled, then those non-rolled up columns
would be treated as metrics and combined.
- There is currently no safeguards (other than that the job will fail
without creating new unexpected segments). We can look into adding these
safeguard later to make the job fail fast.
- Also tested hll, theta, and quantilesDoublesSketch aggregators via the
Integration Tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
