You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/11/09 01:18:23 UTC
[GitHub] [druid] gianm commented on a change in pull request #11895: revert ColumnAnalysis type, add typeSignature and use it for DruidSchema
gianm commented on a change in pull request #11895:
URL: https://github.com/apache/druid/pull/11895#discussion_r745211540
##########
File path: processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java
##########
@@ -73,6 +78,12 @@ public String getType()
return type;
}
+ @JsonProperty
+ public ColumnType getTypeSignature()
Review comment:
Consider placing this getter above `getType`, which I think will put it earlier in the JSON and make it seem more "official".
##########
File path: processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java
##########
@@ -136,7 +147,19 @@ public ColumnAnalysis fold(ColumnAnalysis rhs)
}
if (!type.equals(rhs.getType())) {
- return ColumnAnalysis.error("cannot_merge_diff_types");
+ return ColumnAnalysis.error(
+ StringUtils.format("cannot_merge_diff_types: [%s] and [%s]", type, rhs.getType())
+ );
+ }
+
+ if (!typeSignature.equals(rhs.getTypeSignature())) {
Review comment:
This'll NPE if `typeSignature` is null — i.e. if it came from a server that is on an older version. I think this is OK, since people are supposed to update Brokers after segment-serving-servers, but I'd still feel better if this code was resilient to null `typeSignature`.
Which reminds me: I think we'll also need a trivial change in the cache key to make sure SegmentAnalysis objects from shared caches are not re-used.
##########
File path: docs/querying/segmentmetadataquery.md
##########
@@ -87,9 +87,11 @@ The format of the result is:
} ]
```
-Dimension columns will have type `STRING`, `FLOAT`, `DOUBLE`, or `LONG`.
-Metric columns will have type `FLOAT`, `DOUBLE`, or `LONG`, or the name of the underlying complex type such as `hyperUnique` in case of COMPLEX metric.
-Timestamp column will have type `LONG`.
+All columns will contain a `typeSignature` which is the Druid internal representation of the type information for this column, is what is show in [`INFORMATION_SCHEMA.COLUMNS`](../querying/sql.md#columns-table) table in SQL, and is typically the value used to supply Druid with JSON type information at query or ingest time. This value will be `STRING`, `FLOAT`, `DOUBLE`, `LONG`, or `COMPLEX<typeName>` (e.g. `COMPLEX<hyperUnique>`).
+
+Additionally, columns will have a legacy friendly `type` name. This might match `typeSignature` for some column types (`STRING`, `FLOAT`, `DOUBLE`, or `LONG`) but for COMPLEX columns will only contain the name of the underlying complex type such as `hyperUnique`.
Review comment:
Consider adding:
> New applications should use `typeSignature`, not `type`.
##########
File path: docs/querying/segmentmetadataquery.md
##########
@@ -87,9 +87,11 @@ The format of the result is:
} ]
```
-Dimension columns will have type `STRING`, `FLOAT`, `DOUBLE`, or `LONG`.
-Metric columns will have type `FLOAT`, `DOUBLE`, or `LONG`, or the name of the underlying complex type such as `hyperUnique` in case of COMPLEX metric.
-Timestamp column will have type `LONG`.
+All columns will contain a `typeSignature` which is the Druid internal representation of the type information for this column, is what is show in [`INFORMATION_SCHEMA.COLUMNS`](../querying/sql.md#columns-table) table in SQL, and is typically the value used to supply Druid with JSON type information at query or ingest time. This value will be `STRING`, `FLOAT`, `DOUBLE`, `LONG`, or `COMPLEX<typeName>` (e.g. `COMPLEX<hyperUnique>`).
Review comment:
show -> shown
##########
File path: processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java
##########
@@ -181,6 +205,7 @@ public String toString()
{
return "ColumnAnalysis{" +
"type='" + type + '\'' +
+ ", columnType=" + typeSignature +
Review comment:
Why not call this typeSignature?
--
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