PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar… #1135

swaroopak · 2021-02-08T20:34:04Z

…y indexes

gokceni · 2021-02-09T00:49:55Z

phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterAddCascadeIndexIT.java

+                "      population BIGINT,\n" +
+                "      CONSTRAINT my_pk PRIMARY KEY (state, city)) " + tableDDLOptions);
+
+        if(isViewIndex) {


Is it possible to rename this as isViewScenario?

stoty · 2021-02-09T09:14:39Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 7s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s	The 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 _
+1 💚	mvninstall	15m 34s	4.x passed
+1 💚	compile	1m 6s	4.x passed
+1 💚	checkstyle	1m 13s	4.x passed
+1 💚	javadoc	0m 47s	4.x passed
+0 🆗	spotbugs	3m 17s	phoenix-core in 4.x has 945 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	6m 47s	the patch passed
+1 💚	compile	1m 8s	the patch passed
+1 💚	javac	1m 8s	the patch passed
-1 ❌	checkstyle	1m 17s	phoenix-core: The patch generated 2 new + 2856 unchanged - 1 fixed = 2858 total (was 2857)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 46s	the patch passed
+1 💚	spotbugs	3m 30s	the patch passed
		_ Other Tests _
+1 💚	unit	182m 9s	phoenix-core in the patch passed.
+1 💚	asflicense	0m 10s	The patch does not generate ASF License warnings.
		219m 41s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1135/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#1135
JIRA Issue	PHOENIX-6344
Optional Tests	dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
uname	Linux b3ccb2e616ff 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev/phoenix-personality.sh
git revision	4.x / `0d5bb3e`
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-1135/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
Test Results	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1135/2/testReport/
Max. process+thread count	4928 (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-1135/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.

virajjasani · 2021-02-09T14:59:14Z

@swaroopak one question (not exactly related to this PR):

From getIndexesPTableForCascade(), we have this block:

                // a child view has access to its parents indexes,
                // this if clause ensures we only get the indexes that
                // are only created on the view itself.
                if (index.getIndexType().equals(IndexType.LOCAL)
                        || (isView && index.getTableName().toString().contains("#"))) {
                    indexesPTable.remove(index);
                }

Having # in indexName represents that it is parent index and not view's own index? If so, is that the only way to differentiate it as of now or do we have some attribute?

virajjasani · 2021-02-09T15:03:54Z

I think i got it, it seems we are using CHILD_VIEW_INDEX_NAME_SEPARATOR mostly to derive it and that's the logic we use so far, e.g

        if (tableName.contains(QueryConstants.CHILD_VIEW_INDEX_NAME_SEPARATOR))

Thanks

…y indexes

gokceni

+1 LGTM

stoty · 2021-02-09T21:51:41Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 6s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s	The 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 _
+1 💚	mvninstall	15m 33s	4.x passed
+1 💚	compile	1m 6s	4.x passed
+1 💚	checkstyle	1m 13s	4.x passed
+1 💚	javadoc	0m 48s	4.x passed
+0 🆗	spotbugs	3m 16s	phoenix-core in 4.x has 945 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	6m 48s	the patch passed
+1 💚	compile	1m 5s	the patch passed
+1 💚	javac	1m 5s	the patch passed
-1 ❌	checkstyle	1m 13s	phoenix-core: The patch generated 2 new + 2856 unchanged - 1 fixed = 2858 total (was 2857)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 49s	the patch passed
+1 💚	spotbugs	3m 27s	the patch passed
		_ Other Tests _
+1 💚	unit	182m 43s	phoenix-core in the patch passed.
+1 💚	asflicense	0m 10s	The patch does not generate ASF License warnings.
		220m 9s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1135/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#1135
JIRA Issue	PHOENIX-6344
Optional Tests	dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile
uname	Linux 2718e6bffe72 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev/phoenix-personality.sh
git revision	4.x / `0d5bb3e`
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-1135/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
Test Results	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1135/3/testReport/
Max. process+thread count	5095 (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-1135/3/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.

stoty · 2021-02-09T21:52:31Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 0s	Docker mode activated.
-1 ❌	patch	0m 4s	#1135 does not apply to 4.x. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.

Subsystem	Report/Notes
GITHUB PR	#1135
JIRA Issue	PHOENIX-6344
Console output	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1135/4/console
versions	git=2.17.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

gjacoby126 · 2021-02-09T22:58:37Z

Belated +1

…y indexes (#1135)

swaroopak requested a review from gjacoby126 February 8, 2021 20:34

swaroopak force-pushed the PHOENIX-6344 branch from 27861d2 to 2dac20b Compare February 8, 2021 22:20

swaroopak changed the title ~~PHOENIX-6148: CASCADE on ALTER should NOOP when there are no secondar…~~ PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar… Feb 8, 2021

gokceni reviewed Feb 9, 2021

View reviewed changes

swaroopak force-pushed the PHOENIX-6344 branch from 2dac20b to cc6a2cd Compare February 9, 2021 18:11

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

1c869be

…y indexes

swaroopak force-pushed the PHOENIX-6344 branch from cc6a2cd to 1c869be Compare February 9, 2021 18:13

gokceni approved these changes Feb 9, 2021

View reviewed changes

swaroopak merged commit 883170e into apache:4.x Feb 9, 2021

swaroopak added a commit that referenced this pull request Feb 11, 2021

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

8a8690b

…y indexes (#1135)

asfgit pushed a commit that referenced this pull request Feb 15, 2021

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

f6b030f

…y indexes (#1135)

asfgit pushed a commit that referenced this pull request Oct 30, 2021

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

72f348c

…y indexes (#1135)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar… #1135

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar… #1135

Uh oh!

swaroopak commented Feb 8, 2021

Uh oh!

gokceni Feb 9, 2021

Uh oh!

stoty commented Feb 9, 2021

Uh oh!

virajjasani commented Feb 9, 2021

Uh oh!

virajjasani commented Feb 9, 2021

Uh oh!

gokceni left a comment

Uh oh!

stoty commented Feb 9, 2021

Uh oh!

stoty commented Feb 9, 2021

Uh oh!

gjacoby126 commented Feb 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar… #1135

PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar… #1135

Uh oh!

Conversation

swaroopak commented Feb 8, 2021

Uh oh!

gokceni Feb 9, 2021

Choose a reason for hiding this comment

Uh oh!

stoty commented Feb 9, 2021

Uh oh!

virajjasani commented Feb 9, 2021

Uh oh!

virajjasani commented Feb 9, 2021

Uh oh!

gokceni left a comment

Choose a reason for hiding this comment

Uh oh!

stoty commented Feb 9, 2021

Uh oh!

stoty commented Feb 9, 2021

Uh oh!

gjacoby126 commented Feb 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants