Skip to content

PHOENIX-6454: Add feature to SchemaTool to get the DDL in specificati… - #1233

Merged
swaroopak merged 1 commit into
apache:4.xfrom
swaroopak:6454_add
Jun 11, 2021
Merged

PHOENIX-6454: Add feature to SchemaTool to get the DDL in specificati…#1233
swaroopak merged 1 commit into
apache:4.xfrom
swaroopak:6454_add

Conversation

@swaroopak

Copy link
Copy Markdown
Contributor

…on mode (Addendum)

@swaroopak
swaroopak requested a review from gjacoby126May 14, 2021 17:43
@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec4m 23sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ 4.x Compile Tests _
+0 🆗mvndep5m 15sMaven dependency ordering for branch
+1 💚mvninstall9m 10s4.x passed
+1 💚compile1m 33s4.x passed
+1 💚checkstyle0m 45s4.x passed
+1 💚javadoc1m 1s4.x passed
+0 🆗spotbugs3m 3sphoenix-core in 4.x has 951 extant spotbugs warnings.
+0 🆗spotbugs0m 45sphoenix-tools in 4.x has 5 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 12sMaven dependency ordering for patch
+1 💚mvninstall5m 47sthe patch passed
+1 💚compile1m 33sthe patch passed
+1 💚javac1m 33sthe patch passed
-1 ❌checkstyle0m 34sphoenix-core: The patch generated 16 new + 297 unchanged - 10 fixed = 313 total (was 307)
-1 ❌checkstyle0m 12sphoenix-tools: The patch generated 11 new + 119 unchanged - 2 fixed = 130 total (was 121)
-1 ❌whitespace0m 0sThe patch 2 line(s) with tabs.
+1 💚javadoc0m 57sthe patch passed
+1 💚spotbugs4m 9sthe patch passed
_ Other Tests _
+1 💚unit137m 36sphoenix-core in the patch passed.
-1 ❌unit3m 26sphoenix-tools in the patch failed.
-1 ❌asflicense0m 19sThe patch generated 1 ASF License warnings.
181m 54s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1233
JIRA IssuePHOENIX-6454
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 7ffd420ce25a 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revision4.x / 0e2b826
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-tools.txt
whitespacehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/artifact/yetus-general-check/output/whitespace-tabs.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/artifact/yetus-general-check/output/patch-unit-phoenix-tools.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/testReport/
asflicensehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/artifact/yetus-general-check/output/patch-asflicense-problems.txt
Max. process+thread count5954 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/1/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec5m 38sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ 4.x Compile Tests _
+0 🆗mvndep5m 9sMaven dependency ordering for branch
+1 💚mvninstall10m 27s4.x passed
+1 💚compile1m 39s4.x passed
+1 💚checkstyle0m 44s4.x passed
+1 💚javadoc1m 2s4.x passed
+0 🆗spotbugs3m 18sphoenix-core in 4.x has 951 extant spotbugs warnings.
+0 🆗spotbugs0m 46sphoenix-tools in 4.x has 5 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 11sMaven dependency ordering for patch
+1 💚mvninstall6m 50sthe patch passed
+1 💚compile1m 40sthe patch passed
+1 💚javac1m 40sthe patch passed
-1 ❌checkstyle0m 32sphoenix-core: The patch generated 17 new + 294 unchanged - 13 fixed = 311 total (was 307)
-1 ❌checkstyle0m 11sphoenix-tools: The patch generated 17 new + 112 unchanged - 8 fixed = 129 total (was 120)
-1 ❌whitespace0m 0sThe patch 2 line(s) with tabs.
+1 💚javadoc1m 2sthe patch passed
+1 💚spotbugs4m 27sthe patch passed
_ Other Tests _
-1 ❌unit196m 27sphoenix-core in the patch failed.
-1 ❌unit4m 40sphoenix-tools in the patch failed.
-1 ❌asflicense1m 11sThe patch generated 1 ASF License warnings.
248m 56s
ReasonTests
Failed junit testsphoenix.end2end.AuditLoggingIT
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1233
JIRA IssuePHOENIX-6454
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux cf672e60ed2f 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revision4.x / 69a9ec3
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-tools.txt
whitespacehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/whitespace-tabs.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/patch-unit-phoenix-tools.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/testReport/
asflicensehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/artifact/yetus-general-check/output/patch-asflicense-problems.txt
Max. process+thread count5163 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/2/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@gjacoby126gjacoby126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits

private void runAndVerify(String expected, String baseDDL) throws Exception {
String[] arg = { "-m", "SYNTH", "-d", baseDDL };
String result = runSchemaTool(null, arg);
System.out.println(result);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can cut out the println.

.getFirst()));
pkList.add(cd);
}
for(ColumnDef cd : addStmt.getColumnDefs()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the ALTER tries to add an existing PK column?

@swaroopakswaroopakJun 10, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid point, I am thinking to create a new Jira which will validate if the statements are valid on the mini-cluster (as parsing won't catch this and similar problem) and then only proceed for synthesis. Does that sound okay?

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec4m 41sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ 4.x Compile Tests _
+0 🆗mvndep5m 25sMaven dependency ordering for branch
-1 ❌mvninstall10m 19sroot in 4.x failed.
+1 💚compile1m 41s4.x passed
+1 💚checkstyle0m 48s4.x passed
+1 💚javadoc1m 7s4.x passed
+0 🆗spotbugs3m 23sphoenix-core in 4.x has 951 extant spotbugs warnings.
+0 🆗spotbugs0m 51sphoenix-tools in 4.x has 5 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 13sMaven dependency ordering for patch
-1 ❌mvninstall6m 15sroot in the patch failed.
+1 💚compile1m 46sthe patch passed
+1 💚javac1m 46sthe patch passed
-1 ❌checkstyle0m 32sphoenix-core: The patch generated 15 new + 140 unchanged - 10 fixed = 155 total (was 150)
-1 ❌checkstyle0m 12sphoenix-tools: The patch generated 15 new + 119 unchanged - 2 fixed = 134 total (was 121)
-1 ❌whitespace0m 0sThe patch 2 line(s) with tabs.
+1 💚javadoc1m 8sthe patch passed
+1 💚spotbugs4m 39sthe patch passed
_ Other Tests _
+1 💚unit197m 3sphoenix-core in the patch passed.
-1 ❌unit3m 25sphoenix-tools in the patch failed.
+1 💚asflicense0m 18sThe patch does not generate ASF License warnings.
245m 4s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1233
JIRA IssuePHOENIX-6454
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 81fc8b31281d 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revision4.x / eb8a91e
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
mvninstallhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/branch-mvninstall-root.txt
mvninstallhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/patch-mvninstall-root.txt
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-tools.txt
whitespacehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/whitespace-tabs.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/artifact/yetus-general-check/output/patch-unit-phoenix-tools.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/testReport/
Max. process+thread count5651 (vs. ulimit of 30000)
modulesC: phoenix-core phoenix-tools U: .
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1233/3/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@swaroopak
swaroopak merged commit b42a25b into apache:4.xJun 11, 2021
swaroopak added a commit that referenced this pull request Jun 14, 2021
richardantal pushed a commit to richardantal/phoenix-1 that referenced this pull request Jul 22, 2021
richardantal pushed a commit to richardantal/phoenix-1 that referenced this pull request Jul 22, 2021
richardantal pushed a commit that referenced this pull request Jul 29, 2021
richardantal pushed a commit that referenced this pull request Jul 29, 2021
jpisaac pushed a commit to jpisaac/phoenix that referenced this pull request Jun 10, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@swaroopak@stoty@gjacoby126