[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/213 PHOENIX-2827 Support OFFSET in Calcite-Phoenix You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix offset Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/213.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #213 commit 488259067d5a610efcc946cb92d48070aa4fbdb0 Author: Eric Date: 2016-09-28T21:40:32Z PHOENIX-2827 Support OFFSET in Calcite-Phoenix --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[ https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15536719#comment-15536719 ] ASF GitHub Bot commented on PHOENIX-2827: - GitHub user lomoree opened a pull request: https://github.com/apache/phoenix/pull/213 PHOENIX-2827 Support OFFSET in Calcite-Phoenix You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/phoenix offset Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/213.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #213 commit 488259067d5a610efcc946cb92d48070aa4fbdb0 Author: Eric Date: 2016-09-28T21:40:32Z PHOENIX-2827 Support OFFSET in Calcite-Phoenix > Support OFFSET in Calcite-Phoenix > - > > Key: PHOENIX-2827 > URL: https://issues.apache.org/jira/browse/PHOENIX-2827 > Project: Phoenix > Issue Type: Task >Reporter: Maryann Xue >Assignee: Eric Lomore > Labels: calcite > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (PHOENIX-3341) Data update is not visible to following statements of the same connection due to CalciteSchema caching.
Maryann Xue created PHOENIX-3341: Summary: Data update is not visible to following statements of the same connection due to CalciteSchema caching. Key: PHOENIX-3341 URL: https://issues.apache.org/jira/browse/PHOENIX-3341 Project: Phoenix Issue Type: Sub-task Reporter: Maryann Xue Assignee: Maryann Xue The TableRef object contains a timestamp which will be used for TableScan. The timestamp should be set at the time of the statement being compiled. Now that the table resolving goes from Calcite and CalciteSchema caches TableEntry through the whole connection, the table will not be re-resolved if any previous statement has already resolved it. If a previous statement did an update, the next statement cannot see the update since it's holding a TableRef object containing the old timestamp. The CalciteSchema caching would also be a problem if a table, a view, or a function is modified or dropped. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3341) Data update is not visible to following statements of the same connection due to CalciteSchema caching.
[ https://issues.apache.org/jira/browse/PHOENIX-3341?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15536876#comment-15536876 ] Maryann Xue commented on PHOENIX-3341: -- [~julianhyde], what's your take on this? CatalogReader calls "CalciteSchema.add(String, Table)" every time a table has been resolved. Shall we provide something like a clear() method for CalciteSchema and Phoenix is going to call it at the start of every statement? We could do some fix work for the "data update not visible" issue, but it would still be a problem for dropping or modifying table, functions, etc. > Data update is not visible to following statements of the same connection due > to CalciteSchema caching. > --- > > Key: PHOENIX-3341 > URL: https://issues.apache.org/jira/browse/PHOENIX-3341 > Project: Phoenix > Issue Type: Sub-task >Reporter: Maryann Xue >Assignee: Maryann Xue > Labels: calcite > > The TableRef object contains a timestamp which will be used for TableScan. > The timestamp should be set at the time of the statement being compiled. Now > that the table resolving goes from Calcite and CalciteSchema caches > TableEntry through the whole connection, the table will not be re-resolved if > any previous statement has already resolved it. If a previous statement did > an update, the next statement cannot see the update since it's holding a > TableRef object containing the old timestamp. > The CalciteSchema caching would also be a problem if a table, a view, or a > function is modified or dropped. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PHOENIX-3242) Support CREATE OR REPLACE FUNCTION in Phoenix-Calcite Integration
[ https://issues.apache.org/jira/browse/PHOENIX-3242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15536887#comment-15536887 ] Maryann Xue commented on PHOENIX-3242: -- [~rajeshbabu], turns out that the UPSERT issue and the DROP FUNCTION issue is both due to PHOENIX-3341. I applied a quick fix for UPSERT but couldn't fix DROP FUNCTION. So please revert the changes I had made in my patch in UserDefinedFunctionsIT#testUDFsWithMultipleConnections. > Support CREATE OR REPLACE FUNCTION in Phoenix-Calcite Integration > - > > Key: PHOENIX-3242 > URL: https://issues.apache.org/jira/browse/PHOENIX-3242 > Project: Phoenix > Issue Type: Sub-task >Reporter: Rajeshbabu Chintaguntla >Assignee: Rajeshbabu Chintaguntla > Labels: calcite > Attachments: PHOENIX-3242-WIP-2.patch, PHOENIX-3242-wip.patch, > PHOENIX-3242_select-wip.patch, PHOENIX-3242_v1.patch, PHOENIX-3242_v2.patch, > PHOENIX-3242_v3.patch, PHOENIX-3242_v4.patch, PHOENIX-3242_v5.patch, > PHOENIX-3242_v6.patch > > -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81419580
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
}
@Override
public QueryPlan implement(PhoenixRelImplementor implementor) {
QueryPlan plan = implementor.visitInput(0, (PhoenixQueryRel)
getInput());
int fetchValue = RexLiteral.intValue(fetch);
+int offsetValue = 0;
+if (offset != null){
+offsetValue = RexLiteral.intValue(offset);
+}
if (plan.getLimit() == null) {
-return plan.limit(fetchValue);
+return plan.limit(fetchValue, offsetValue);
}
return new ClientScanPlan(plan.getContext(), plan.getStatement(),
implementor.getTableMapping().getTableRef(),
RowProjector.EMPTY_PROJECTOR,
-fetchValue, null, null, OrderBy.EMPTY_ORDER_BY, plan);
+fetchValue, offsetValue, null, OrderBy.EMPTY_ORDER_BY,
plan);
--- End diff --
Since either of them could be null, Boolean objects should be used instead
of primitive boolean type.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81420342
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
I can see there's a big confusion here. Basically, offset and limit should
be treated the same way. So here, it should be "if both the old limit and the
old offset are same as as the new ones specified, we return this; otherwise we
create a new object."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81419366
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
}
@Override
public QueryPlan implement(PhoenixRelImplementor implementor) {
QueryPlan plan = implementor.visitInput(0, (PhoenixQueryRel)
getInput());
int fetchValue = RexLiteral.intValue(fetch);
--- End diff --
I believe now that we've enabled offset, "this.fetch" could be also be
null, which means it should be handled the same way as this.offset now. Could
you please also add test cases to cover situations like "offset != null" but
"limit == null"?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81419684
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rules/PhoenixConverterRules.java
---
@@ -232,8 +232,7 @@ public RelNode convert(RelNode rel) {
private static Predicate HAS_FETCH = new
Predicate() {
@Override
public boolean apply(LogicalSort input) {
-return input.offset == null
-&& input.fetch != null;
+return input.fetch != null;
--- End diff --
should be "input.fetch != null || input.offset != null"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81418958
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
}
@Override
public QueryPlan implement(PhoenixRelImplementor implementor) {
QueryPlan plan = implementor.visitInput(0, (PhoenixQueryRel)
getInput());
int fetchValue = RexLiteral.intValue(fetch);
+int offsetValue = 0;
+if (offset != null){
+offsetValue = RexLiteral.intValue(offset);
+}
if (plan.getLimit() == null) {
--- End diff --
should be "if (plan.getLimit() == null && plan.getOffset() == null)"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81418723
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
--- End diff --
I'd write:
int offset = this.offset == null ? 0 : RexLiteral.intValue(this.offset);
return Math.min(RexLiteral.intValue(fetch), rows - offset);
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537114#comment-15537114
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81418958
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
}
@Override
public QueryPlan implement(PhoenixRelImplementor implementor) {
QueryPlan plan = implementor.visitInput(0, (PhoenixQueryRel)
getInput());
int fetchValue = RexLiteral.intValue(fetch);
+int offsetValue = 0;
+if (offset != null){
+offsetValue = RexLiteral.intValue(offset);
+}
if (plan.getLimit() == null) {
--- End diff --
should be "if (plan.getLimit() == null && plan.getOffset() == null)"
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537113#comment-15537113
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81419684
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rules/PhoenixConverterRules.java
---
@@ -232,8 +232,7 @@ public RelNode convert(RelNode rel) {
private static Predicate HAS_FETCH = new
Predicate() {
@Override
public boolean apply(LogicalSort input) {
-return input.offset == null
-&& input.fetch != null;
+return input.fetch != null;
--- End diff --
should be "input.fetch != null || input.offset != null"
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537112#comment-15537112
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81420342
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
I can see there's a big confusion here. Basically, offset and limit should
be treated the same way. So here, it should be "if both the old limit and the
old offset are same as as the new ones specified, we return this; otherwise we
create a new object."
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537111#comment-15537111
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81418723
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
--- End diff --
I'd write:
int offset = this.offset == null ? 0 : RexLiteral.intValue(this.offset);
return Math.min(RexLiteral.intValue(fetch), rows - offset);
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537109#comment-15537109
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81419580
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
}
@Override
public QueryPlan implement(PhoenixRelImplementor implementor) {
QueryPlan plan = implementor.visitInput(0, (PhoenixQueryRel)
getInput());
int fetchValue = RexLiteral.intValue(fetch);
+int offsetValue = 0;
+if (offset != null){
+offsetValue = RexLiteral.intValue(offset);
+}
if (plan.getLimit() == null) {
-return plan.limit(fetchValue);
+return plan.limit(fetchValue, offsetValue);
}
return new ClientScanPlan(plan.getContext(), plan.getStatement(),
implementor.getTableMapping().getTableRef(),
RowProjector.EMPTY_PROJECTOR,
-fetchValue, null, null, OrderBy.EMPTY_ORDER_BY, plan);
+fetchValue, offsetValue, null, OrderBy.EMPTY_ORDER_BY,
plan);
--- End diff --
Since either of them could be null, Boolean objects should be used instead
of primitive boolean type.
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537110#comment-15537110
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81419366
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rel/PhoenixLimit.java ---
@@ -76,20 +76,27 @@ public RelOptCost computeSelfCost(RelOptPlanner
planner, RelMetadataQuery mq) {
@Override
public double estimateRowCount(RelMetadataQuery mq) {
-double rows = super.estimateRowCount(mq);
+double rows = super.estimateRowCount(mq);
+if(offset != null) {
+return Math.max(0, Math.min(RexLiteral.intValue(fetch), rows -
RexLiteral.intValue(offset)));
+}
return Math.min(RexLiteral.intValue(fetch), rows);
}
@Override
public QueryPlan implement(PhoenixRelImplementor implementor) {
QueryPlan plan = implementor.visitInput(0, (PhoenixQueryRel)
getInput());
int fetchValue = RexLiteral.intValue(fetch);
--- End diff --
I believe now that we've enabled offset, "this.fetch" could be also be
null, which means it should be handled the same way as this.offset now. Could
you please also add test cases to cover situations like "offset != null" but
"limit == null"?
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81421343
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
This does definitely have ripple effects throughout, but I had done this on
the basis that SQL only allows an offset if their is an imposed limit (since an
offset doesn't make any logical sense without a limit).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537126#comment-15537126
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81421343
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
This does definitely have ripple effects throughout, but I had done this on
the basis that SQL only allows an offset if their is an imposed limit (since an
offset doesn't make any logical sense without a limit).
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537155#comment-15537155
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81422631
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rules/PhoenixConverterRules.java
---
@@ -232,8 +232,7 @@ public RelNode convert(RelNode rel) {
private static Predicate HAS_FETCH = new
Predicate() {
@Override
public boolean apply(LogicalSort input) {
-return input.offset == null
-&& input.fetch != null;
+return input.fetch != null;
--- End diff --
With respect to my comment below, if there is a limit, then we apply the
rule, otherwise we do not (regardless of offset).
I may have the wrong idea here, but based on my understanding of offset,
all of the test cases pass as expected. Are the test cases I have laid out in
CalciteIT what you would expect?
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81422631
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/calcite/rules/PhoenixConverterRules.java
---
@@ -232,8 +232,7 @@ public RelNode convert(RelNode rel) {
private static Predicate HAS_FETCH = new
Predicate() {
@Override
public boolean apply(LogicalSort input) {
-return input.offset == null
-&& input.fetch != null;
+return input.fetch != null;
--- End diff --
With respect to my comment below, if there is a limit, then we apply the
rule, otherwise we do not (regardless of offset).
I may have the wrong idea here, but based on my understanding of offset,
all of the test cases pass as expected. Are the test cases I have laid out in
CalciteIT what you would expect?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81430086
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
Is it a SQL-92 standard that OFFSET can only be used together with LIMIT? I
know there are some dialects that allow different OFFSET/LIMIT or OFFSET/FETCH
grammars, so I'm just confused... I tried with Calcite parser and it does allow
OFFSET without LIMIT.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537289#comment-15537289
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81430086
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
Is it a SQL-92 standard that OFFSET can only be used together with LIMIT? I
know there are some dialects that allow different OFFSET/LIMIT or OFFSET/FETCH
grammars, so I'm just confused... I tried with Calcite parser and it does allow
OFFSET without LIMIT.
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81430788
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
For right or wrong, Phoenix allows an OFFSET to be used without a LIMIT
today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537308#comment-15537308
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81430788
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
For right or wrong, Phoenix allows an OFFSET to be used without a LIMIT
today.
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[GitHub] phoenix pull request #213: PHOENIX-2827 Support OFFSET in Calcite-Phoenix
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81431821
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
Ah, my mistake. I'll get working on a patch that also supports offset
without limit. This should incorporate all of the changes noted above. Thanks
for clarifying.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[jira] [Commented] (PHOENIX-2827) Support OFFSET in Calcite-Phoenix
[
https://issues.apache.org/jira/browse/PHOENIX-2827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537344#comment-15537344
]
ASF GitHub Bot commented on PHOENIX-2827:
-
Github user lomoree commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/213#discussion_r81431821
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/execute/ClientScanPlan.java ---
@@ -106,12 +106,18 @@ public ExplainPlan getExplainPlan() throws
SQLException {
}
@Override
-public QueryPlan limit(Integer limit) {
-if (limit == this.limit || (limit != null &&
limit.equals(this.limit)))
+public QueryPlan limit(Integer limit, Integer offset) {
+if (limit == this.limit || (limit != null &&
limit.equals(this.limit))) {
--- End diff --
Ah, my mistake. I'll get working on a patch that also supports offset
without limit. This should incorporate all of the changes noted above. Thanks
for clarifying.
> Support OFFSET in Calcite-Phoenix
> -
>
> Key: PHOENIX-2827
> URL: https://issues.apache.org/jira/browse/PHOENIX-2827
> Project: Phoenix
> Issue Type: Task
>Reporter: Maryann Xue
>Assignee: Eric Lomore
> Labels: calcite
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PHOENIX-3264) Allow TRUE and FALSE to be used as literal constants
[ https://issues.apache.org/jira/browse/PHOENIX-3264?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15537670#comment-15537670 ] James Taylor commented on PHOENIX-3264: --- Nice job, [~lomoree] & [~maryannxue] - this fixed 92 unit tests! I think this JIRA can be closed as resolved now too. > Allow TRUE and FALSE to be used as literal constants > > > Key: PHOENIX-3264 > URL: https://issues.apache.org/jira/browse/PHOENIX-3264 > Project: Phoenix > Issue Type: Sub-task >Reporter: James Taylor >Assignee: Eric Lomore > Attachments: Sql2RelImplementation.png, SqlLiteral.png, > SqlNodeToRexConverterImpl.png, SqlOptionNode.png, objectdependencies.png, > objectdependencies2.png, stacktrace.png > > > Phoenix supports TRUE and FALSE as boolean literals, but perhaps Calcite > doesn't? Looks like this is leading to a fair number of failures. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Resolved] (PHOENIX-3264) Allow TRUE and FALSE to be used as literal constants
[ https://issues.apache.org/jira/browse/PHOENIX-3264?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Maryann Xue resolved PHOENIX-3264. -- Resolution: Fixed Yes, [~jamestaylor], was going to do in the afternoon but our home network was broken... Anyway, thank you very much for the nice work, [~lomoree]! > Allow TRUE and FALSE to be used as literal constants > > > Key: PHOENIX-3264 > URL: https://issues.apache.org/jira/browse/PHOENIX-3264 > Project: Phoenix > Issue Type: Sub-task >Reporter: James Taylor >Assignee: Eric Lomore > Attachments: Sql2RelImplementation.png, SqlLiteral.png, > SqlNodeToRexConverterImpl.png, SqlOptionNode.png, objectdependencies.png, > objectdependencies2.png, stacktrace.png > > > Phoenix supports TRUE and FALSE as boolean literals, but perhaps Calcite > doesn't? Looks like this is leading to a fair number of failures. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
