Skip to content

PHOENIX-6124 : Conditionally block adding/dropping a column on a parent view - #917

Closed
virajjasani wants to merge 3 commits into
apache:masterfrom
virajjasani:PHOENIX-6124-master
Closed

PHOENIX-6124 : Conditionally block adding/dropping a column on a parent view#917
virajjasani wants to merge 3 commits into
apache:masterfrom
virajjasani:PHOENIX-6124-master

Conversation

@virajjasani

Copy link
Copy Markdown
Contributor

No description provided.

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec5m 49sDocker 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.
_ master Compile Tests _
+1 💚mvninstall45m 0smaster passed
+1 💚compile1m 2smaster passed
+1 💚checkstyle0m 57smaster passed
+1 💚javadoc0m 48smaster passed
+0 🆗spotbugs3m 11sphoenix-core in master has 973 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall43m 7sthe patch passed
+1 💚compile0m 55sthe patch passed
+1 💚javac0m 55sthe patch passed
-1 ❌checkstyle0m 59sphoenix-core: The patch generated 33 new + 1711 unchanged - 24 fixed = 1744 total (was 1735)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 45sthe patch passed
+1 💚spotbugs3m 21sthe patch passed
_ Other Tests _
-1 ❌unit147m 34sphoenix-core in the patch failed.
+1 💚asflicense0m 21sThe patch does not generate ASF License warnings.
256m 12s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#917
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux f676ad21cdc7 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / 4b2ff49
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-917/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/1/testReport/
Max. process+thread count6128 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/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 🆗reexec1m 3sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 1sNo 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.
_ master Compile Tests _
+1 💚mvninstall44m 59smaster passed
+1 💚compile1m 2smaster passed
+1 💚checkstyle0m 59smaster passed
+1 💚javadoc0m 47smaster passed
+0 🆗spotbugs3m 7sphoenix-core in master has 973 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall41m 45sthe patch passed
+1 💚compile0m 57sthe patch passed
+1 💚javac0m 57sthe patch passed
-1 ❌checkstyle0m 58sphoenix-core: The patch generated 31 new + 1713 unchanged - 22 fixed = 1744 total (was 1735)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 46sthe patch passed
+1 💚spotbugs3m 21sthe patch passed
_ Other Tests _
-1 ❌unit150m 33sphoenix-core in the patch failed.
+1 💚asflicense0m 21sThe patch does not generate ASF License warnings.
253m 6s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#917
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux a989c9a5796d 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / b4afc08
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-917/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/2/testReport/
Max. process+thread count6216 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/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.

@virajjasani

Copy link
Copy Markdown
ContributorAuthor

It's strange that build data is not present over ci-hadoop jenkins.

@stoty

Copy link
Copy Markdown
Contributor

It is indeed.
Jenkins reports that no builds have happened, even though we can clearly see that they did, and the above report was generated.
The same happened for PR 918, and 919.
I haven't touched this precommit job config in weeks.
If this keeps happening, we'll need to ask for help from INFRA.

@virajjasani

Copy link
Copy Markdown
ContributorAuthor

Thanks @stoty for taking a look.

@virajjasani

Copy link
Copy Markdown
ContributorAuthor

INFRA Jira is filed. FYI https://issues.apache.org/jira/browse/INFRA-20984

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 48sDocker 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.
_ master Compile Tests _
+1 💚mvninstall47m 34smaster passed
+1 💚compile1m 18smaster passed
+1 💚checkstyle1m 4smaster passed
+1 💚javadoc1m 0smaster passed
+0 🆗spotbugs3m 40sphoenix-core in master has 973 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall47m 45sthe patch passed
+1 💚compile1m 2sthe patch passed
+1 💚javac1m 2sthe patch passed
-1 ❌checkstyle1m 5sphoenix-core: The patch generated 31 new + 1713 unchanged - 22 fixed = 1744 total (was 1735)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 58sthe patch passed
+1 💚spotbugs3m 37sthe patch passed
_ Other Tests _
-1 ❌unit166m 46sphoenix-core in the patch failed.
+1 💚asflicense0m 23sThe patch does not generate ASF License warnings.
280m 31s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#917
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux f389f4655a3e 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / 5b2eeb6
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-917/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/3/testReport/
Max. process+thread count6239 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/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.

@virajjasani

Copy link
Copy Markdown
ContributorAuthor

@ChinmaySKulkarni@gjacoby126 could you please review this PR?
Recent build has no test failure and I have tried to fix max checkstyles except for the ones related to method length, hope that should be fine.

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 6sDocker 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.
_ master Compile Tests _
+1 💚mvninstall46m 26smaster passed
+1 💚compile1m 2smaster passed
+1 💚checkstyle0m 56smaster passed
+1 💚javadoc0m 49smaster passed
+0 🆗spotbugs3m 13sphoenix-core in master has 973 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall41m 33sthe patch passed
+1 💚compile0m 57sthe patch passed
+1 💚javac0m 57sthe patch passed
-1 ❌checkstyle0m 58sphoenix-core: The patch generated 7 new + 1711 unchanged - 24 fixed = 1718 total (was 1735)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 47sthe patch passed
+1 💚spotbugs3m 19sthe patch passed
_ Other Tests _
-1 ❌unit148m 19sphoenix-core in the patch failed.
+1 💚asflicense0m 26sThe patch does not generate ASF License warnings.
252m 19s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#917
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 1f9a9136fcf7 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / fe7c46c
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-917/4/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/4/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/4/testReport/
Max. process+thread count6219 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/4/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 🆗reexec1m 9sDocker 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.
_ master Compile Tests _
-1 ❌mvninstall46m 16sroot in master failed.
+1 💚compile1m 2smaster passed
+1 💚checkstyle0m 58smaster passed
+1 💚javadoc0m 46smaster passed
+0 🆗spotbugs3m 7sphoenix-core in master has 973 extant spotbugs warnings.
_ Patch Compile Tests _
-1 ❌mvninstall41m 22sroot in the patch failed.
+1 💚compile0m 56sthe patch passed
+1 💚javac0m 56sthe patch passed
-1 ❌checkstyle0m 58sphoenix-core: The patch generated 7 new + 1711 unchanged - 24 fixed = 1718 total (was 1735)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 44sthe patch passed
+1 💚spotbugs3m 21sthe patch passed
_ Other Tests _
-1 ❌unit150m 24sphoenix-core in the patch failed.
+1 💚asflicense0m 22sThe patch does not generate ASF License warnings.
253m 51s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/5/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#917
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 3f2981beb574 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / 628fa0d
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-917/5/artifact/yetus-general-check/output/branch-mvninstall-root.txt
mvninstallhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/5/artifact/yetus-general-check/output/patch-mvninstall-root.txt
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/5/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/5/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/5/testReport/
Max. process+thread count6207 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/5/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.

@yanxinyi

Copy link
Copy Markdown
Contributor

LGTM +1.

@BeforeClass
public static synchronized void doSetup() throws Exception {
Map<String, String> serverProps = Maps.newHashMapWithExpectedSize(1);
serverProps.put(QueryServices.ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK, "true");

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.

I think Phoenix already tested add/dropColumnOnParentTableView when ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK=false so no dup tests are needed.
Just want to double confirm :)

@virajjasanivirajjasaniOct 17, 2020

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.

Thanks @yanxinyi , I think there are tests related to ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK=false (default value) but CANNOT_MUTATE_TABLE error code is not tested for table and view both. Also, without this source code change, this test will fail because we don't have coverage for View. Hence, thought of writing it separately to cover all 4 cases together: add/delete and table/view.
Hence, I think for false value of ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK, we don't need additional test :)
Sounds good?

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec8m 39sDocker 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.
_ master Compile Tests _
+1 💚mvninstall17m 58smaster passed
+1 💚compile1m 17smaster passed
+1 💚checkstyle1m 14smaster passed
+1 💚javadoc1m 2smaster passed
+0 🆗spotbugs3m 51sphoenix-core in master has 973 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall12m 47sthe patch passed
+1 💚compile1m 22sthe patch passed
+1 💚javac1m 22sthe patch passed
-1 ❌checkstyle1m 15sphoenix-core: The patch generated 5 new + 1713 unchanged - 22 fixed = 1718 total (was 1735)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc1m 0sthe patch passed
+1 💚spotbugs3m 42sthe patch passed
_ Other Tests _
-1 ❌unit162m 44sphoenix-core in the patch failed.
+1 💚asflicense0m 24sThe patch does not generate ASF License warnings.
219m 50s
SubsystemReport/Notes
DockerClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/6/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#917
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
unameLinux 90d9d1536968 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 toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / 4703e46
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-917/6/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/6/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/6/testReport/
Max. process+thread count6161 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-917/6/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.

@virajjasani
virajjasani deleted the PHOENIX-6124-master branch October 20, 2020 17:18
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

@virajjasani@stoty@yanxinyi