clintropolis commented on code in PR #20593:
URL: https://github.com/apache/druid/pull/20593#discussion_r4278883068
##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidUnionDataSourceRule.java:
##########
@@ -122,88 +108,50 @@ public static boolean isCompatible(
return false;
}
- if (!unionRel.all && null != plannerContext) {
- plannerContext.setPlanningError("SQL requires 'UNION' but only 'UNION
ALL' is supported.");
- }
- return unionRel.all && isUnionCompatible(first, second, plannerContext);
- }
-
- private static boolean isUnionCompatible(final DruidRel<?> first, final
DruidRel<?> second, @Nullable PlannerContext plannerContext)
- {
- final Optional<List<String>> firstColumnNames =
getColumnNamesIfTableOrUnion(first, plannerContext);
- final Optional<List<String>> secondColumnNames =
getColumnNamesIfTableOrUnion(second, plannerContext);
- if (!firstColumnNames.isPresent() || !secondColumnNames.isPresent()) {
- // No need to set the planning error here
- return false;
- }
- if (!firstColumnNames.equals(secondColumnNames)) {
+ if (!unionRel.all) {
if (null != plannerContext) {
- plannerContext.setPlanningError("SQL requires union between two tables
and column names queried for " +
- "each table are different Left: %s, Right: %s.",
- firstColumnNames.orElse(Collections.emptyList()),
- secondColumnNames.orElse(Collections.emptyList()));
+ plannerContext.setPlanningError("SQL requires 'UNION' but only 'UNION
ALL' is supported.");
}
return false;
}
- return true;
+
+ return isUnionArm(first, plannerContext) && isUnionArm(second,
plannerContext);
}
- static Optional<List<String>> getColumnNamesIfTableOrUnion(final DruidRel<?>
druidRel, @Nullable PlannerContext plannerContext)
+ /**
+ * Whether a single input can be an arm of a {@link DruidUnionDataSourceRel}.
+ */
+ static boolean isUnionArm(final DruidRel<?> druidRel, @Nullable
PlannerContext plannerContext)
{
- final PartialDruidQuery partialQuery = druidRel.getPartialDruidQuery();
+ // A union input is flattened into the new union.
+ final boolean isUnion = druidRel instanceof DruidUnionDataSourceRel;
- final Optional<DruidTable> druidTable =
+ final boolean isTable =
DruidRels.druidTableIfLeafRel(druidRel)
- .filter(table -> table.getDataSource() instanceof
TableDataSource);
-
- if (druidTable.isPresent() && DruidRels.isScanOrMapping(druidRel, false)) {
- // This rel is a table scan or mapping.
-
- if (partialQuery.stage() == PartialDruidQuery.Stage.SCAN) {
- return
Optional.of(druidTable.get().getRowSignature().getColumnNames());
- } else {
- // Sanity check. Expected to be true due to the "scan or mapping"
check.
- if (partialQuery.stage() != PartialDruidQuery.Stage.SELECT_PROJECT) {
- throw new ISE("Expected stage %s but got %s",
PartialDruidQuery.Stage.SELECT_PROJECT, partialQuery.stage());
- }
-
- // Apply the mapping (with additional sanity checks).
- final RowSignature tableSignature = druidTable.get().getRowSignature();
- final Mappings.TargetMapping mapping =
partialQuery.getSelectProject().getMapping();
-
- if (mapping.getSourceCount() != tableSignature.size()) {
- throw new ISE(
- "Expected mapping with %d columns but got %d columns",
- tableSignature.size(),
- mapping.getSourceCount()
- );
- }
-
- final List<String> retVal = new ArrayList<>();
-
- for (int i = 0; i < mapping.getTargetCount(); i++) {
- final int sourceField = mapping.getSourceOpt(i);
- retVal.add(tableSignature.getColumnName(sourceField));
- }
-
- return Optional.of(retVal);
- }
- } else if (!druidTable.isPresent() && druidRel instanceof
DruidUnionDataSourceRel) {
- // This rel is a union itself.
+ .filter(table -> table.getDataSource() instanceof
TableDataSource)
+ .isPresent();
- return Optional.of(((DruidUnionDataSourceRel)
druidRel).getUnionColumnNames());
- } else if (druidTable.isPresent()) {
- if (null != plannerContext) {
- plannerContext.setPlanningError("SQL requires union between inputs
that are not simple table scans " +
- "and involve a filter or aliasing. Or column types of tables being
unioned are not of same type.");
- }
- return Optional.empty();
- } else {
+ if (!isUnion && !isTable) {
if (null != plannerContext) {
plannerContext.setPlanningError("SQL requires union with input of a
datasource type that is not supported."
+ " Union operation is only supported between regular tables. ");
}
- return Optional.empty();
+ return false;
+ }
+
+ // Flattening a union discards anything layered on top of it, so there
must be nothing.
+ final boolean isSimple = isUnion
+ ? druidRel.getPartialDruidQuery().stage() ==
PartialDruidQuery.Stage.SCAN
+ : DruidRels.isScanOrFilterOrProject(druidRel);
+
+ if (!isSimple) {
+ if (null != plannerContext) {
+ plannerContext.setPlanningError("SQL requires union between inputs
that are not simple table scans "
+ + "and involve more than a filter or a projection.");
Review Comment:
nit: this error message reads a bit off
--
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]
