You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/11/30 17:44:40 UTC
[GitHub] [phoenix] virajjasani opened a new pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
virajjasani opened a new pull request #990:
URL: https://github.com/apache/phoenix/pull/990
* ExplainPlan can provide plan as attributes object for simplified assertion of individual plan attributes
* Provide an additional API to get plan object
* Explain queries can still continue to provide output as String / list of String, object based approach is just an additional parameter for better comparison
* Utilizes object based comparison in BaseStatsCollectorIT as per Jira, the plan is to extend object based comparison to maximum tests with further sub-tasks of [PHOENIX-5265](https://issues.apache.org/jira/browse/PHOENIX-5265)
* Support HashJoinPlan as part of separate Jira
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #990:
URL: https://github.com/apache/phoenix/pull/990#issuecomment-737036166
Thanks for the review @stoty
> Does the plan involve removing the existing line-by-line generation and building the String format plan form the new object at some point ?
Indeed I think we can give this a shot in long term for sure. That's also partly the reason why I have kept separate new API which takes object builder as input rather than updating current API which just takes list of String to accumulate entire plan step by step.
Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #990:
URL: https://github.com/apache/phoenix/pull/990#issuecomment-736099322
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 49s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 3 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 15m 25s | master passed |
| +1 :green_heart: | compile | 1m 4s | master passed |
| +1 :green_heart: | checkstyle | 2m 38s | master passed |
| +1 :green_heart: | javadoc | 1m 0s | master passed |
| +0 :ok: | spotbugs | 3m 56s | phoenix-core in master has 966 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 8m 51s | the patch passed |
| +1 :green_heart: | compile | 0m 54s | the patch passed |
| +1 :green_heart: | javac | 0m 54s | the patch passed |
| -1 :x: | checkstyle | 2m 33s | phoenix-core: The patch generated 508 new + 4831 unchanged - 85 fixed = 5339 total (was 4916) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 44s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 6s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 100m 3s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 29s | The patch does not generate ASF License warnings. |
| | | 144m 24s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.end2end.ConcurrentMutationsExtendedIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/990 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux be2522e1ed7f 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / 993ad5e |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/2/testReport/ |
| Max. process+thread count | 6687 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/2/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #990:
URL: https://github.com/apache/phoenix/pull/990#issuecomment-737036878
> This also seems to be low risk, as it doesn't change existing functionality.
That's correct, and that's the reason this should not cause any regression.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #990:
URL: https://github.com/apache/phoenix/pull/990#issuecomment-736017138
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 3 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 13m 16s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | checkstyle | 2m 5s | master passed |
| +1 :green_heart: | javadoc | 0m 47s | master passed |
| +0 :ok: | spotbugs | 3m 5s | phoenix-core in master has 966 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 7m 47s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| -1 :x: | checkstyle | 2m 52s | phoenix-core: The patch generated 508 new + 4831 unchanged - 85 fixed = 5339 total (was 4916) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 52s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 27s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 109m 37s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 26s | The patch does not generate ASF License warnings. |
| | | 149m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/990 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 14da9d27d30c 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / 993ad5e |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/1/testReport/ |
| Max. process+thread count | 6836 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-990/1/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #990:
URL: https://github.com/apache/phoenix/pull/990#issuecomment-737031495
+1
I have skimmed the patch, and I like the general idea.
This also seems to be low risk, as it doesn't change existing functionality.
Does the plan involve removing the existing line-by-line generation and building the String format plan form the new object at some point ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty closed pull request #990: PHOENIX-5728 : ExplainPlan with plan as attributes object
Posted by GitBox <gi...@apache.org>.
stoty closed pull request #990:
URL: https://github.com/apache/phoenix/pull/990
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org