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 2021/02/08 20:34:15 UTC

[GitHub] [phoenix] swaroopak opened a new pull request #1135: PHOENIX-6148: CASCADE on ALTER should NOOP when there are no secondar…

swaroopak opened a new pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135


   …y indexes


----------------------------------------------------------------
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 #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#issuecomment-775787461


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  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 :x: |  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 :green_heart: |  mvninstall  |  15m 34s |  4.x passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  4.x passed  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  4.x passed  |
   | +1 :green_heart: |  javadoc  |   0m 47s |  4.x passed  |
   | +0 :ok: |  spotbugs  |   3m 17s |  phoenix-core in 4.x has 945 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 47s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  8s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  8s |  the patch passed  |
   | -1 :x: |  checkstyle  |   1m 17s |  phoenix-core: The patch generated 2 new + 2856 unchanged - 1 fixed = 2858 total (was 2857)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m 46s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   3m 30s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 182m  9s |  phoenix-core in the patch passed.  |
   | +1 :green_heart: |  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 | https://github.com/apache/phoenix/pull/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.
   
   


----------------------------------------------------------------
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 #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#issuecomment-776002281


   @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?


----------------------------------------------------------------
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] gjacoby126 commented on pull request #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#issuecomment-776300527


   Belated +1


----------------------------------------------------------------
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 #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#issuecomment-776267777


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  6s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  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 :x: |  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 :green_heart: |  mvninstall  |  15m 33s |  4.x passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  4.x passed  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  4.x passed  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  4.x passed  |
   | +0 :ok: |  spotbugs  |   3m 16s |  phoenix-core in 4.x has 945 extant spotbugs warnings.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 48s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | -1 :x: |  checkstyle  |   1m 13s |  phoenix-core: The patch generated 2 new + 2856 unchanged - 1 fixed = 2858 total (was 2857)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  the patch passed  |
   | +1 :green_heart: |  spotbugs  |   3m 27s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 182m 43s |  phoenix-core in the patch passed.  |
   | +1 :green_heart: |  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 | https://github.com/apache/phoenix/pull/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.
   
   


----------------------------------------------------------------
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] gokceni commented on a change in pull request #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
gokceni commented on a change in pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#discussion_r572485854



##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterAddCascadeIndexIT.java
##########
@@ -172,6 +172,41 @@ public void testAlterDBOAddCascadeIndexAllUpsert() throws Exception {
 
     }
 
+    // Test with ALTER TABLE/VIEW CASCADE INDEX ALL with no indexes
+    @Test
+    public void testAlterDBOAddCascadeIndexAll_noIndexes() throws Exception {
+        String schemaName = "S_"+generateUniqueName();
+        String tableName = "T_"+generateUniqueName();
+        String viewName = "V_"+generateUniqueName();
+
+        String fullViewName = SchemaUtil.getQualifiedTableName(schemaName, viewName);
+        String fullTableName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+
+        conn.createStatement().execute("CREATE TABLE IF NOT EXISTS " + fullTableName + " (\n" +
+                "      state CHAR(2) NOT NULL,\n" +
+                "      city VARCHAR NOT NULL,\n" +
+                "      population BIGINT,\n" +
+                "      CONSTRAINT my_pk PRIMARY KEY (state, city)) " + tableDDLOptions);
+
+        if(isViewIndex) {

Review comment:
       Is it possible to rename this as isViewScenario?




----------------------------------------------------------------
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 #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#issuecomment-776005755


   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


----------------------------------------------------------------
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 #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135#issuecomment-776268155


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m  0s |  Docker mode activated.  |
   | -1 :x: |  patch  |   0m  4s |  https://github.com/apache/phoenix/pull/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 | https://github.com/apache/phoenix/pull/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.
   
   


----------------------------------------------------------------
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] swaroopak merged pull request #1135: PHOENIX-6344: CASCADE on ALTER should NOOP when there are no secondar…

Posted by GitBox <gi...@apache.org>.
swaroopak merged pull request #1135:
URL: https://github.com/apache/phoenix/pull/1135


   


----------------------------------------------------------------
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