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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
