[
https://issues.apache.org/jira/browse/PHOENIX-1580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14392124#comment-14392124
]
James Taylor edited comment on PHOENIX-1580 at 4/2/15 4:31 AM:
---------------------------------------------------------------
Looks good, [~ayingshu]. Some minor stuff:
- Remove this function from UnionCompiler as it's not called and not needed:
{code}
+ public static void checkForOrderByLimitInUnionAllSelect(SelectStatement
select) throws SQLException {
+ if (select.getOrderBy() != null && !select.getOrderBy().isEmpty()) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.ORDER_BY_IN_UNIONALL_SELECT_NOT_SUPPORTED).setMessage(".").build().buildException();
+ }
+ if (select.getLimit() != null) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.LIMIT_IN_UNIONALL_SELECT_NOT_SUPPORTED).setMessage(".").build().buildException();
+ }
+ }
+
{code}
- Add static constant for "unionAllTable".getBytes() in UnionCompiler
- Remove these from SQLExceptionCode:
{code}
+ ORDER_BY_IN_UNIONALL_SELECT_NOT_SUPPORTED(523, "42900", "ORDER BY in a
Union All query is not allowed"),
+ LIMIT_IN_UNIONALL_SELECT_NOT_SUPPORTED(524, "42901", "LIMIT in a Union
All query is not allowed"),
{code}
- Implement the methods in UnionPlan more appropriately:
- Don't store private List<QueryPlan> plans, but instead have a final
ResultIterators iterators and initialize it to new
UnionResultIterators(this.getPlans()) in the constructor
- Implement getSplits() as iterators.getSplits() and getScans() as
iterators.getScans().
- Implement getTableRef() as returning either null or by returning a static
final constant TableRef for union. Seems like you wouldn't need to pass this
through the constructor as it doesn't seem like it would ever be different.
- Declare final boolean isDegenerate and calculate it in the constructor by
looping through all plans and if any are not context.getScanRanges() ==
ScanRanges.NOTHING, then stop the loop and set this.isDegenarate to false. The
implement isDegenerate() as returning this.isDegenerate.
- Add a step at the beginning of the UnionPlan.getExplainPlan() like "UNION
" + iterators.size() + " queries\n"
- Remove this code from UnionPlan.getExplainPlan():
{code}
+ if (context.getSequenceManager().getSequenceCount() > 0) {
+ int nSequences = context.getSequenceManager().getSequenceCount();
+ steps.add("CLIENT RESERVE VALUES FROM " + nSequences + " SEQUENCE"
+ (nSequences == 1 ? "" : "S"));
+ }
{code}
- Instead of looping through plans in UnionPlan.explain(), call
iterators.explain(steps).
- In UnionResultIterators, do all the work in the constructor - you don't want
to have to construct a list with each call to getScans() or getRanges().
- No need to copy the List<QueryPlans> in UnionResultIterators. Just set the
final List<QueryPlan> plans.
- Remove this check from UnionResultIterators.explain() as it's not necessary:
if (iterators != null && !iterators.isEmpty()). Implement it like this instead:
{code}
+ for (QueryPlan plan : plans) {
+ List<String> planSteps = plan.getExplainPlan().getPlanSteps();
+ steps.addAll(planSteps);
+ }
{code}
was (Author: jamestaylor):
Looks good, [~ayingshu]. Some minor stuff:
- Remove this function from UnionCompiler as it's not called and not needed:
{code}
+ public static void checkForOrderByLimitInUnionAllSelect(SelectStatement
select) throws SQLException {
+ if (select.getOrderBy() != null && !select.getOrderBy().isEmpty()) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.ORDER_BY_IN_UNIONALL_SELECT_NOT_SUPPORTED).setMessage(".").build().buildException();
+ }
+ if (select.getLimit() != null) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.LIMIT_IN_UNIONALL_SELECT_NOT_SUPPORTED).setMessage(".").build().buildException();
+ }
+ }
+
{code}
- Add static constant for "unionAllTable".getBytes() in UnionCompiler
- Remove these from SQLExceptionCode:
{code}
+ ORDER_BY_IN_UNIONALL_SELECT_NOT_SUPPORTED(523, "42900", "ORDER BY in a
Union All query is not allowed"),
+ LIMIT_IN_UNIONALL_SELECT_NOT_SUPPORTED(524, "42901", "LIMIT in a Union
All query is not allowed"),
{code}
- Implement the methods in UnionPlan more appropriately:
- Don't store private List<QueryPlan> plans, but instead have a final
ResultIterators iterators and initialize it to new
UnionResultIterators(this.getPlans()) in the constructor
- Implement getSplits() as iterators.getSplits() and getScans() as
iterators.getScans().
- Implement getTableRef() as returning either null or by returning a static
final constant TableRef for union. Seems like you wouldn't need to pass this
through the constructor as it doesn't seem like it would ever be different.
- Declare final boolean isDegenerate and calculate it in the constructor by
looping through all plans and if any are not context.getScanRanges() ==
ScanRanges.NOTHING, then stop the loop and set this.isDegenarate to false. The
implement isDegenerate() as returning this.isDegenerate.
- Add a step at the beginning of the UnionPlan.getExplainPlan() like "UNION " +
iterators.size() + " queries\n"
- Remove this code from UnionPlan.getExplainPlan():
{code}
+ if (context.getSequenceManager().getSequenceCount() > 0) {
+ int nSequences = context.getSequenceManager().getSequenceCount();
+ steps.add("CLIENT RESERVE VALUES FROM " + nSequences + " SEQUENCE"
+ (nSequences == 1 ? "" : "S"));
+ }
{code}
- Instead of looping through plans in UnionPlan.explain(), call
iterators.explain(steps).
- In UnionResultIterators, do all the work in the constructor - you don't want
to have to construct a list with each call to getScans() or getRanges().
- No need to copy the List<QueryPlans> in UnionResultIterators. Just set the
final List<QueryPlan> plans.
- Remove this check from UnionResultIterators.explain() as it's not necessary:
if (iterators != null && !iterators.isEmpty()). Implement it like this instead:
{code}
+ for (QueryPlan plan : plans) {
+ List<String> planSteps = plan.getExplainPlan().getPlanSteps();
+ steps.addAll(planSteps);
+ }
{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-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)
