gjacoby126 commented on a change in pull request #473: PHOENIX-5138 - 
ViewIndexId sequences created after PHOENIX-5132 shoul…
URL: https://github.com/apache/phoenix/pull/473#discussion_r270581752
 
 

 ##########
 File path: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
 ##########
 @@ -2310,6 +2316,98 @@ private static void mapChildViewsToNamespace(String 
connUrl, Properties props, L
         }
     }
 
+    public static void mergeViewIndexIdSequences(ConnectionQueryServices cqs, 
PhoenixConnection metaConnection)
+        throws SQLException{
+         /* before PHOENIX-5132, there was a per-tenant sequence to generate 
view index ids,
+           which could cause problems if global and tenant-owned view indexes 
were mixed for the
+           same physical base table. Now there's just one sequence for all 
view indexes of the same
+           physical table, but we have to check to see if there are any legacy 
sequences, and
+           merge them into a single sequence equal to max + 1 of the largest 
legacy sequence
+           to avoid collisons.
+         */
+        Map<String, List<SequenceKey>> sequenceTableMap = new HashMap<>();
+        DatabaseMetaData metaData = metaConnection.getMetaData();
+
+        try (ResultSet sequenceRS = metaData.getTables(null, null,
+            "%" + MetaDataUtil.VIEW_INDEX_SEQUENCE_NAME_PREFIX + "%",
+            new String[] {PhoenixDatabaseMetaData.SEQUENCE_TABLE_TYPE})) {
+            while (sequenceRS.next()) {
+                String tenantId = sequenceRS.getString(TABLE_CAT);
+                String schemaName = sequenceRS.getString(TABLE_SCHEM);
+                String sequenceName = sequenceRS.getString(TABLE_NAME);
+                int numBuckets = sequenceRS.getInt(SALT_BUCKETS);
+                SequenceKey key = new SequenceKey(tenantId, schemaName, 
sequenceName, numBuckets);
+                String baseTableName;
+                //under the old naming convention, view index sequences
+                // of non-namespace mapped tables stored their physical table 
name in the sequence schema for
+                //some reason. Namespace-mapped tables stored it in the 
sequence name itself.
+                //Note the difference between VIEW_INDEX_SEQUENCE_PREFIX 
(_SEQ_)
+                //and VIEW_INDEX_SEQUENCE_NAME_PREFIX (_ID_)
+                if (schemaName != null && 
schemaName.contains(MetaDataUtil.VIEW_INDEX_SEQUENCE_PREFIX)) {
+                    baseTableName = 
schemaName.replace(MetaDataUtil.VIEW_INDEX_SEQUENCE_PREFIX, "");
+                } else {
+                    baseTableName = SchemaUtil.getTableName(schemaName,
+                        
sequenceName.replace(MetaDataUtil.VIEW_INDEX_SEQUENCE_NAME_PREFIX, ""));
+                }
+                if (!sequenceTableMap.containsKey(baseTableName)) {
+                    sequenceTableMap.put(baseTableName, new 
ArrayList<SequenceKey>());
+                }
+                sequenceTableMap.get(baseTableName).add(key);
+            }
+        }
+        for (String baseTableName : sequenceTableMap.keySet()){
+            Map<SequenceKey, Long> currentSequenceValues = new 
HashMap<SequenceKey, Long>();
+            long maxViewIndexId = Long.MIN_VALUE;
+            PName name = PNameFactory.newName(baseTableName);
+            boolean hasNamespaceMapping =
+                SchemaUtil.isNamespaceMappingEnabled(null, 
cqs.getConfiguration()) ||
+                    
cqs.getProps().getBoolean(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, false);
+            List<SequenceKey> existingSequenceKeys = 
sequenceTableMap.get(baseTableName);
+            for (SequenceKey sequenceKey : existingSequenceKeys){
+                long[] currentValueArray = new long[1];
+                SQLException[] sqlExceptions = new SQLException[1];
+                cqs.incrementSequences(
+                    Lists.newArrayList(new SequenceAllocation(sequenceKey, 
1L)),
+                    EnvironmentEdgeManager.currentTimeMillis(),
+                    currentValueArray, new SQLException[1]);
+
+                if (sqlExceptions[0] != null) {
+                    continue;
 
 Review comment:
   @twdsilva -- yeah, I'd thought of that and was torn. On the one hand, 
skipping it like I'm doing here can lead to collisions in edge cases. On the 
other hand, I didn't want to permanently crash an upgrade if someone had 
exhausted a single sequence. Happy to hear suggestions. :-)
   
   Maybe only continue for certain kinds of exceptions? If so, which ones?

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to