zachjsh commented on code in PR #14560:
URL: https://github.com/apache/druid/pull/14560#discussion_r1261524567
##########
processing/src/main/java/org/apache/druid/query/metadata/metadata/SegmentMetadataQuery.java:
##########
@@ -106,11 +107,28 @@ public SegmentMetadataQuery(
this.toInclude = toInclude == null ? new AllColumnIncluderator() :
toInclude;
this.merge = merge == null ? false : merge;
this.analysisTypes = analysisTypes;
- Preconditions.checkArgument(
- dataSource instanceof TableDataSource || dataSource instanceof
UnionDataSource,
- "SegmentMetadataQuery only supports table or union datasource"
- );
- this.lenientAggregatorMerge = lenientAggregatorMerge == null ? false :
lenientAggregatorMerge;
+ if (!(dataSource instanceof TableDataSource || dataSource instanceof
UnionDataSource)) {
+ throw InvalidInput.exception("Invalid dataSource type [%s]. "
+ + "SegmentMetadataQuery only supports table
or union datasources.", dataSource);
+ }
+ // We validate that there's only one parameter specified by the user.
While the deprecated property is still
+ // supported in the API, we only set the new member variable either using
old or new property, so we've a single source
+ // of truth for consumers of this class variable. The defaults are to
preserve backwards compatibility.
+ // In a future release, 28.0+, we can remove the deprecated property
lenientAggregatorMerge.
+ if (lenientAggregatorMerge != null && aggregatorMergeStrategy != null) {
Review Comment:
If both are specified, but consistent, should we allow?
--
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]
