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 2022/04/13 13:29:32 UTC

[GitHub] [phoenix] tkhurana commented on a diff in pull request #1413: PHOENIX-6681 Optionally disable indexes during creation

tkhurana commented on code in PR #1413:
URL: https://github.com/apache/phoenix/pull/1413#discussion_r849475818


##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -2874,7 +2877,17 @@ public boolean isViewReferenced() {
             Collections.reverse(columnMetadata);
             tableMetaData.addAll(columnMetadata);
             String dataTableName = parent == null || tableType == PTableType.VIEW ? null : parent.getTableName().getString();
-            PIndexState indexState = parent == null || tableType == PTableType.VIEW  ? null : PIndexState.BUILDING;
+            PIndexState defaultCreateState = PIndexState.valueOf(connection.getQueryServices().getConfiguration().
+                            get(INDEX_CREATE_DEFAULT_STATE, QueryServicesOptions.DEFAULT_CREATE_INDEX_STATE));
+            if (defaultCreateState == PIndexState.CREATE_DISABLE) {
+                if  (indexType == IndexType.LOCAL || sharedTable) {

Review Comment:
   A comment would be useful here why we are not using create_disable state for view indexes



##########
phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java:
##########
@@ -232,7 +232,15 @@ public static void serialize(PTable dataTable, ImmutableBytesWritable ptr,
         }
         int nIndexes = indexes.size();
         if (dataTable.getTransformingNewTable() != null) {
-            nIndexes++;
+            // If the transforming new table is in CREATE_DISABLE state, the mutations don't go into the table.
+            boolean disabled = dataTable.getTransformingNewTable().isIndexStateDisabled();
+            if (disabled && nIndexes == 0) {
+                ptr.set(ByteUtil.EMPTY_BYTE_ARRAY);

Review Comment:
   A log here will be helpful



##########
phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java:
##########
@@ -248,11 +256,15 @@ public static void serialize(PTable dataTable, ImmutableBytesWritable ptr,
                     output.write(protoBytes);
             }
             if (dataTable.getTransformingNewTable() != null) {
-                ServerCachingProtos.TransformMaintainer proto = TransformMaintainer.toProto(
-                        dataTable.getTransformingNewTable().getTransformMaintainer(dataTable, connection));
-                byte[] protoBytes = proto.toByteArray();
-                WritableUtils.writeVInt(output, protoBytes.length);
-                output.write(protoBytes);
+                // We're not serializing the TransformMaintainer if the new transformed table is disabled
+                boolean disabled = dataTable.getTransformingNewTable().isIndexStateDisabled();
+                if (!disabled) {
+                    ServerCachingProtos.TransformMaintainer proto = TransformMaintainer.toProto(
+                            dataTable.getTransformingNewTable().getTransformMaintainer(dataTable, connection));
+                    byte[] protoBytes = proto.toByteArray();
+                    WritableUtils.writeVInt(output, protoBytes.length);
+                    output.write(protoBytes);
+                }

Review Comment:
   we could log in an `else` new transform table is disabled



##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -2874,7 +2877,17 @@ public boolean isViewReferenced() {
             Collections.reverse(columnMetadata);
             tableMetaData.addAll(columnMetadata);
             String dataTableName = parent == null || tableType == PTableType.VIEW ? null : parent.getTableName().getString();
-            PIndexState indexState = parent == null || tableType == PTableType.VIEW  ? null : PIndexState.BUILDING;
+            PIndexState defaultCreateState = PIndexState.valueOf(connection.getQueryServices().getConfiguration().
+                            get(INDEX_CREATE_DEFAULT_STATE, QueryServicesOptions.DEFAULT_CREATE_INDEX_STATE));
+            if (defaultCreateState == PIndexState.CREATE_DISABLE) {
+                if  (indexType == IndexType.LOCAL || sharedTable) {
+                    defaultCreateState = PIndexState.BUILDING;
+                }
+            }
+            PIndexState indexState = parent == null || tableType == PTableType.VIEW  ? null : defaultCreateState;
+            if (indexState == null && tableProps.containsKey(INDEX_STATE)) {
+                indexState = PIndexState.fromSerializedValue(tableProps.get(INDEX_STATE).toString());

Review Comment:
   I am guessing this is needed for transform table. It will be useful to add a comment



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

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org