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 in
the same metric name column) 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). Also 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 preserved + 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]
