Fix the alias handling in single-stage engine #11610
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11610 +/- ##
=============================================
- Coverage 67.28% 14.49% -52.79%
+ Complexity 905 201 -704
=============================================
Files 1731 2325 +594
Lines 90155 124913 +34758
Branches 14440 19146 +4706
=============================================
- Hits 60661 18109 -42552
- Misses 25406 105276 +79870
+ Partials 4088 1528 -2560
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2109 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Jackie-Jiang
force-pushed
the
fix_alias
branch
from
efa1e8a
to
de9456b
Compare
Jackie-Jiang
added
the
incompatible
| aliasMap.put(identifierExpr.getIdentifier(), functionCall.getOperands().get(0)); | ||
| String alias = function.getOperands().get(1).getIdentifier().getName(); | ||
| if (aliasMap.put(alias, function.getOperands().get(0)) != null) { | ||
| throw new SqlCompilationException("Find duplicate alias: " + alias); |
| public static final List<String> DEFAULT_QUERY_REWRITERS_CLASS_NAMES = | ||
| ImmutableList.of(CompileTimeFunctionsInvoker.class.getName(), SelectionsRewriter.class.getName(), | ||
| PredicateComparisonRewriter.class.getName(), OrdinalsUpdater.class.getName(), | ||
| AliasApplier.class.getName(), NonAggregationGroupByToDistinctQueryRewriter.class.getName()); | ||
| PredicateComparisonRewriter.class.getName(), AliasApplier.class.getName(), OrdinalsUpdater.class.getName(), |
| } | ||
| } | ||
| return aliasMap; | ||
| } | ||
|
|
||
| private static void applyAlias(Map<Identifier, Expression> aliasMap, PinotQuery pinotQuery) { | ||
| Expression filterExpression = pinotQuery.getFilterExpression(); |
There was a problem hiding this comment.
My concern here is that this will silently return empty results once users upgrade.
Shall we instead throw exception if detected the alias in filter and explicitly mention that alias shouldn't be used, so users can at least find out the root cause and either roll back or fix queries before upgrade?
xiangfu0
force-pushed
the
fix_alias
branch
4 times, most recently
from
e8314ee
to
ac7e28f
Compare
|
Just for curiosity, what regression do you observe when the #10815 is introduced? |
master branch build failure: |
Fix 3 issues:
Incompatible
Alias can no longer be applied to filter to comply with standard SQL