[
https://issues.apache.org/jira/browse/PHOENIX-1580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14393020#comment-14393020
]
James Taylor commented on PHOENIX-1580:
---------------------------------------
Thanks for the revisions, [~ayingshu]. Here's some more feedback:
- Passing through the tableRef to UnionPlan as you're doing is fine.
- In UnionCompiler, create static constant final PName member variables for the
dummy table name and schema name so that you're not instantiating these again
and again. I'd recommend calling them UNION_FAMILY_NAME, UNION_TABLE_NAME, and
UNION_SCHEMA_NAME. We'll use the same names when UNION is implemented versus
UNION ALL, so no need to have ALL in them.
{code}
+ public static TableRef contructSchemaTable(PhoenixStatement statement,
QueryPlan plan) throws SQLException {
+ List<PColumn> projectedColumns = new ArrayList<PColumn>();
+ for (int i=0; i< plan.getProjector().getColumnCount(); i++) {
+ ColumnProjector colProj =
plan.getProjector().getColumnProjector(i);
+ Expression sourceExpression = colProj.getExpression();
+ PColumnImpl projectedColumn = new
PColumnImpl(PNameFactory.newName(colProj.getName().getBytes()),
PNameFactory.newName(DUMMY_FAMILY.getBytes()),
+ sourceExpression.getDataType(),
sourceExpression.getMaxLength(), sourceExpression.getScale(),
sourceExpression.isNullable(),
+ i, sourceExpression.getSortOrder(), 500, null, false,
sourceExpression.toString());
+ projectedColumns.add(projectedColumn);
+ }
+ Long scn = statement.getConnection().getSCN();
+ PTable tempTable =
PTableImpl.makePTable(statement.getConnection().getTenantId(),
PNameFactory.newName(UNION_ALL_SCHEMA.getBytes()),
PNameFactory.newName(UNION_ALL_TABLE.getBytes()),
+ PTableType.SUBQUERY, null, HConstants.LATEST_TIMESTAMP, scn ==
null ? HConstants.LATEST_TIMESTAMP : scn, null, null, projectedColumns, null,
null, null,
+ true, null, null, null, true, true, true, null, null,
null);
+ TableRef tableRef = new TableRef(null, tempTable, 0, false);
+ return tableRef;
+ }
{code}
- In the above code, construct your PName like this as it prevents a String
from being created again:
{code}
new PColumnImpl(PNameFactory.newName(colProj.getName())
{code}
- In UnionPlan, declare these member variables as constants. When you calculate
isDegenerate just use a local variable in the constructor and initialize the
member variables once with the final result:
{code}
+ private boolean isDegenerate = true;
+ private final List<QueryPlan> plans;
{code}
- For the explain plan, you've essentially implemented part of the logic twice,
in two different ways. Once in UnionPlan.getExplainPlan() where you loop over
the query plans:
{code}
+ @Override
+ public ExplainPlan getExplainPlan() throws SQLException {
+ List<String> steps = new ArrayList<String>();
+ steps.add("UNION ALL " + iterators.size() + " queries\n");
+ for (QueryPlan plan : this.getPlans()) {
+ List<String> planSteps = plan.getExplainPlan().getPlanSteps();
+ steps.addAll(planSteps);
+ }
{code}
And then the same logic again in UnionResultIterators.explain():
{code}
+ @Override
+ public void explain(List<String> planSteps) {
+ for (int index=0; index < iterators.size(); index++) {
+ iterators.get(index).explain(planSteps);
+ }
+ }
{code}
Although both are correct, the former is much easier to understand. So I
recommend you implement explain like this instead:
{code}
+ @Override
+ public void explain(List<String> planSteps) {
+ for (QueryPlan plan : plans) {
+ List<String> planSteps = plan.getExplainPlan().getPlanSteps();
+ steps.addAll(planSteps);
+ }
+ }
{code}
and modify the UnionPlan.getExplainPlan() to delegate to UnionResultIterators
as you're already doing for many of the calls:
{code}
+ @Override
+ public ExplainPlan getExplainPlan() throws SQLException {
+ List<String> steps = new ArrayList<String>();
+ steps.add("UNION ALL " + iterators.size() + " queries\n");
+ iterators.explain(steps);
+ if (!orderBy.getOrderByExpressions().isEmpty()) {
+ steps.add("CLIENT" + (limit == null ? "" : " TOP " + limit + "
ROW" + (limit == 1 ? "" : "S")) + " SORTED BY " +
orderBy.getOrderByExpressions().toString());
+ } else if (limit != null) {
+ steps.add("CLIENT " + limit + " ROW LIMIT");
+ }
+
+ return new ExplainPlan(steps);
+ }
{code}
- In UnionResultIterators, size all the lists in the constructor, as you have a
good idea of what the size will be:
{code}
+public class UnionResultIterators implements ResultIterators {
+ private final List<KeyRange> splits = new ArrayList<KeyRange>();
+ private final List<List<Scan>> scans = new ArrayList<List<Scan>>();
+ private final List<PeekingResultIterator> iterators = new
ArrayList<PeekingResultIterator>();
+ private final List<QueryPlan> plans;
+
+ public UnionResultIterators(List<QueryPlan> plans) throws SQLException {
+ this.plans = plans;
+ for (QueryPlan plan : this.plans) {
+ iterators.add(LookAheadResultIterator.wrap(plan.iterator()));
+ if (plan.getSplits() != null)
+ splits.addAll(plan.getSplits());
+ if (plan.getScans() != null)
+ scans.addAll(plan.getScans());
+ }
+ }
+
{code}
> Support UNION ALL
> -----------------
>
> Key: PHOENIX-1580
> URL: https://issues.apache.org/jira/browse/PHOENIX-1580
> Project: Phoenix
> Issue Type: Bug
> Reporter: Alicia Ying Shu
> Assignee: Alicia Ying Shu
> Attachments: PHOENIX-1580-grammar.patch, Phoenix-1580-v1.patch,
> Phoenix-1580-v2.patch, Phoenix-1580-v3.patch, Phoenix-1580-v4.patch,
> Phoenix-1580-v5.patch, Phoenix-1580-v6.patch, phoenix-1580-v1-wipe.patch,
> phoenix-1580.patch, unionall-wipe.patch
>
>
> Select * from T1
> UNION ALL
> Select * from T2
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
