gianm commented on code in PR #19162:
URL: https://github.com/apache/druid/pull/19162#discussion_r2941576151
##########
server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java:
##########
@@ -991,11 +991,6 @@ static RowSignature analysisToRowSignature(final
SegmentAnalysis analysis)
{
final RowSignature.Builder rowSignatureBuilder = RowSignature.builder();
for (Map.Entry<String, ColumnAnalysis> entry :
analysis.getColumns().entrySet()) {
- if (entry.getValue().isError()) {
Review Comment:
In particular, on your historicals or realtime tasks do you see warnings
from `SegmentAnalyzer` like `Error analyzing column` or `Unknown column type`?
If we can fix those then the type would be correct here too, rather than
getting reset to `STRING`.
##########
server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java:
##########
@@ -991,11 +991,6 @@ static RowSignature analysisToRowSignature(final
SegmentAnalysis analysis)
{
final RowSignature.Builder rowSignatureBuilder = RowSignature.builder();
for (Map.Entry<String, ColumnAnalysis> entry :
analysis.getColumns().entrySet()) {
- if (entry.getValue().isError()) {
Review Comment:
I'm surprised you're facing analysis errors when `merge` is off. Most
analysis errors I've seen are related to incompatible types in `fold`, which
won't happen here since we aren't doing `merge`. I wonder if we should fix the
underlying probably that causes the errors to happen… do you have more info
about that?
##########
server/src/test/java/org/apache/druid/segment/metadata/CoordinatorSegmentMetadataCacheTest.java:
##########
@@ -1190,6 +1190,50 @@ public void testSegmentMetadataFallbackType()
);
}
+ /**
+ * Verifies that columns with analysis errors are included in the row
signature with {@link ColumnType#UNKNOWN_COMPLEX}
Review Comment:
I believe errors show up as `STRING` not `COMPLEX`.
--
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]
