You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@phoenix.apache.org by xjodoin <gi...@git.apache.org> on 2018/05/23 17:32:04 UTC
[GitHub] phoenix pull request #301: PHOENIX-4728 The upsert select must project tuple...
GitHub user xjodoin opened a pull request:
https://github.com/apache/phoenix/pull/301
PHOENIX-4728 The upsert select must project tuples
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xjodoin/phoenix PHOENIX-4728
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/phoenix/pull/301.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 #301
----
commit 0411931b6e25aec2d5825fd6d5b538a145558b6a
Author: Xavier Jodoin <xa...@...>
Date: 2018-05-23T17:29:43Z
PHOENIX-4728 The upsert select must project tuples
----
---
[GitHub] phoenix issue #301: PHOENIX-4728 The upsert select must project tuples
Posted by xjodoin <gi...@git.apache.org>.
Github user xjodoin commented on the issue:
https://github.com/apache/phoenix/pull/301
No the problem come from the querycompiler for the upsertselect the behavior is different than a simple select query
Le 23 mai 2018 16:04:46 HAE, James Taylor <no...@github.com> a écrit :
>JamesRTaylor commented on this pull request.
>
>
>
>> @@ -549,7 +549,7 @@ public MutationPlan compile(UpsertStatement
>upsert) throws SQLException {
> select = SelectStatement.create(select, hint);
>// Pass scan through if same table in upsert and select so that
>projection is computed correctly
> // Use optimizer to choose the best plan
>- QueryCompiler compiler = new QueryCompiler(statement,
>select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new
>SequenceManager(statement), false, false, null);
>+ QueryCompiler compiler = new QueryCompiler(statement,
>select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new
>SequenceManager(statement), true, false, null);
>
>This seems like too general of a change for the specific issue you're
>trying to fix for ARRAY_APPEND. I'm also not sure *why* it would impact
>it. Can't you make changes to ArrayAppendFunction or it's base class to
>get the desired affect?
>
>Any opinions, @maryannxue. Do you remember when/why we need this
>projectTuples boolean for QueryCompiler?
>
>--
>You are receiving this because you authored the thread.
>Reply to this email directly or view it on GitHub:
>https://github.com/apache/phoenix/pull/301#pullrequestreview-122745792
--
Envoyé de mon appareil Android avec K-9 Mail. Veuillez excuser ma brièveté.
---
[GitHub] phoenix pull request #301: PHOENIX-4728 The upsert select must project tuple...
Posted by maryannxue <gi...@git.apache.org>.
Github user maryannxue commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/301#discussion_r190428434
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java ---
@@ -549,7 +549,7 @@ public MutationPlan compile(UpsertStatement upsert) throws SQLException {
select = SelectStatement.create(select, hint);
// Pass scan through if same table in upsert and select so that projection is computed correctly
// Use optimizer to choose the best plan
- QueryCompiler compiler = new QueryCompiler(statement, select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new SequenceManager(statement), false, false, null);
+ QueryCompiler compiler = new QueryCompiler(statement, select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new SequenceManager(statement), true, false, null);
--- End diff --
"Tuple projection" was first used for join queries so that columns are accessed based on positions instead of names. We later applied this to single-table queries, but for some reason (reason that I can't recall right now), we wanted to avoid tuple projection in UPSERT. If this change won't cause any existing test failure, I think it's just fine.
---
[GitHub] phoenix pull request #301: PHOENIX-4728 The upsert select must project tuple...
Posted by xjodoin <gi...@git.apache.org>.
Github user xjodoin commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/301#discussion_r190875305
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java ---
@@ -549,7 +549,7 @@ public MutationPlan compile(UpsertStatement upsert) throws SQLException {
select = SelectStatement.create(select, hint);
// Pass scan through if same table in upsert and select so that projection is computed correctly
// Use optimizer to choose the best plan
- QueryCompiler compiler = new QueryCompiler(statement, select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new SequenceManager(statement), false, false, null);
+ QueryCompiler compiler = new QueryCompiler(statement, select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new SequenceManager(statement), true, false, null);
--- End diff --
All the tests passed
---
[GitHub] phoenix pull request #301: PHOENIX-4728 The upsert select must project tuple...
Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/301#discussion_r190381688
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java ---
@@ -549,7 +549,7 @@ public MutationPlan compile(UpsertStatement upsert) throws SQLException {
select = SelectStatement.create(select, hint);
// Pass scan through if same table in upsert and select so that projection is computed correctly
// Use optimizer to choose the best plan
- QueryCompiler compiler = new QueryCompiler(statement, select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new SequenceManager(statement), false, false, null);
+ QueryCompiler compiler = new QueryCompiler(statement, select, selectResolver, targetColumns, parallelIteratorFactoryToBe, new SequenceManager(statement), true, false, null);
--- End diff --
This seems like too general of a change for the specific issue you're trying to fix for ARRAY_APPEND. I'm also not sure *why* it would impact it. Can't you make changes to ArrayAppendFunction or it's base class to get the desired affect?
Any opinions, @maryannxue. Do you remember when/why we need this projectTuples boolean for QueryCompiler?
---