You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "abhishekrb19 (via GitHub)" <gi...@apache.org> on 2026/03/16 01:59:05 UTC
[PR] Preserve row signature column order when column analysis has errors (druid)
abhishekrb19 opened a new pull request, #19162:
URL: https://github.com/apache/druid/pull/19162
Fixes https://github.com/apache/druid/issues/18437 and relates to https://github.com/apache/druid/pull/18966.
When column analysis encounters errors during merge, the current behavior can cause row signatures to flap on the Brokers, which in turn leads to sporadic query failures or incorrect query results, since query plans rely on the Broker’s segment metadata cache. This issue is more pronounced during segment analysis on realtime servers with JSON columns, where the merge may sometimes produce errors, presumably due to type coercion.
This patch ensures that columns are not skipped when such errors occur preserving the row signature's order.
### Release note
Preserve row signature column order when column analysis encounters errors, preventing schema flapping and sporadic query failures or incorrect results (fixes https://github.com/apache/druid/issues/18437)
This PR has:
- [x] been self-reviewed.
- [x] a release note entry in the PR description.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [x] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Preserve row signature column order when column analysis has errors (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #19162:
URL: https://github.com/apache/druid/pull/19162#discussion_r2937810194
##########
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()) {
- // Skip columns with analysis errors.
- continue;
- }
-
ColumnType valueType = entry.getValue().getTypeSignature();
// this shouldn't happen, but if it does, first try to fall back to legacy type information field in case
Review Comment:
I think it’s also possible to simplify the surrounding code as the comments note. However, it may be better to do that separately but happy to bundle it into this bug fix as well if folks think it's safe to do so.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Preserve row signature column order when column analysis has errors (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #19162:
URL: https://github.com/apache/druid/pull/19162#discussion_r2944542998
##########
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:
> As for why this happens here, I’m not entirely sure....but these were happening only on the realtime tasks in our case.
Ah, they're probably trying to merge the analyses from different persists. Are you using auto typing? Either `type: auto` for a dimension spec or `useSchemaDiscovery: true`? I wonder if there is some issue when different persists select different types for the same column. That kind of thing is generally reconciled at query time and also reconciled when the segment was built and merged, but maybe we missed a place where it needs to be reconciled in the `segmentMetadata` path.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Preserve row signature column order when column analysis has errors (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #19162:
URL: https://github.com/apache/druid/pull/19162#discussion_r2944491179
##########
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:
Thanks for taking a look!
Yes, these analysis errors are indeed coming from `fold`. I think I got tripped up by the error logs I had added: `Folding column[xyz] is an error[error:cannot_merge_diff_types: [json] and [STRING]] for mergeStrategy[strict] dataSources[abc]`.
As for why this happens here, I’m not entirely sure....but these were happening only on the realtime tasks in our case. So I had a suspicion that the data for some of these JSON columns in the ingestion spec were numeric strings/null/sparse data and contributing to these analysis errors `[cannot_merge_diff_types](error:cannot_merge_diff_types: [json] and [STRING]])` .
1. For the unstable signature issue, this can cause spurious query failures or return no data. Unfortunately, there’s no good workaround other than forcing a reorder of the columns in the ingestion spec, so this fix should help address it.
2. For any type-related correctness issues, I think users can likely work around them with some form of casting in the queries. (I hadn't seen this issue reported by users so far)
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Preserve row signature column order when column analysis has errors (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Preserve row signature column order when column analysis has errors (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #19162:
URL: https://github.com/apache/druid/pull/19162#discussion_r2944491179
##########
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:
Thanks for taking a look!
Yes, these analysis errors are indeed coming from `fold`. I think I got tripped up by the error logs I had added: `Folding column[xyz] is an error[error:cannot_merge_diff_types: [json] and [STRING]] for mergeStrategy[strict] dataSources[abc]`.
As for why this happens here, I’m not entirely sure....but these were happening only on the realtime tasks in our case. So I had a suspicion that the data for some of these JSON columns in the ingestion spec were numeric strings/null/sparse data and contributing to these analysis errors `[cannot_merge_diff_types](error:cannot_merge_diff_types: [json] and [STRING]])` .
1. For the unstable signature issue, this can cause spurious query failures or return no data. Unfortunately, there’s no good workaround other than forcing a reorder of the columns, so this fix should help address it.
2. For any type-related correctness issues, I think users can likely work around them with some form of casting in the queries. (I hadn't seen this issue reported by users so far)
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org