[GitHub] [phoenix] joshelser commented on a change in pull request #1365: PHOENIX-6611 Multiple IndexTool bugs

[GitBox <gi...@apache.org>]

joshelser commented on a change in pull request #1365:
URL: https://github.com/apache/phoenix/pull/1365#discussion_r777753055



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexTool.java
##########
@@ -667,7 +672,12 @@ private Job configureJobForAsyncIndex() throws Exception {
                 try {
                     admin = pConnection.getQueryServices().getAdmin();
                     TableName hDdataTableName = TableName.valueOf(pDataTable.getPhysicalName().getBytes());
-                    snapshotName = new StringBuilder(hDdataTableName.toString()).append("-Snapshot").toString();
+                    snapshotName = new StringBuilder("INDEXTOOL-")
+                            .append(pDataTable.getName().getString())
+                            .append("-Snapshot-")
+                            .append(System.currentTimeMillis())
+                            .toString();
+                    //FIXME Drop this snapshot after we're done ?

Review comment:
       👍🏼 

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/transform/TransformMaintainer.java
##########
@@ -133,6 +140,30 @@ private TransformMaintainer(final PTable oldTable, final PTable newTable, Phoeni
         this.oldTableEncodingScheme = oldTable.getEncodingScheme() == null ? PTable.QualifierEncodingScheme.NON_ENCODED_QUALIFIERS : oldTable.getEncodingScheme();
         this.oldTableImmutableStorageScheme = oldTable.getImmutableStorageScheme() == null ? PTable.ImmutableStorageScheme.ONE_CELL_PER_COLUMN : oldTable.getImmutableStorageScheme();
 
+        try {
+            // If the old table has global indexes, and does not have the legacy mutable indexing 
+            // coprocessor, then set the empty column value to VERIFIED.
+            // This should take care of tables with immutable, transactional, and new style mutable 
+            // indexes
+            boolean hasGlobalIndex = false;
+            for (PTable oldIndexTable : oldTable.getIndexes()) {
+                if (oldIndexTable.getIndexType() == IndexType.GLOBAL) {
+                    hasGlobalIndex = true;
+                }
+            }
+            if (hasGlobalIndex) {
+                ConnectionQueryServices queryServices = connection.unwrap(PhoenixConnection.class).getQueryServices();
+                HTable oldHBaseTable = (HTable) queryServices.getTable(oldTable.getPhysicalName().getBytes());
+                if (!oldHBaseTable.getDescriptor().hasCoprocessor(org.apache.phoenix.hbase.index.Indexer.class.getName())) {
+                    newTableIndexVerified = true;
+                }
+            }
+        } catch (IOException e) {
+            throw new IllegalArgumentException("Could not determine empty column value", e);
+        } catch (SQLException e) {

Review comment:
       nit: mulit-catch `catch (IOException | SQLException e) {...`




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