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/07/16 14:35:49 UTC

[GitHub] [phoenix] virajjasani commented on a change in pull request #1261: PHOENIX-6506 : Tenant Connection is not able to access/validate Global Sequences

virajjasani commented on a change in pull request #1261:
URL: https://github.com/apache/phoenix/pull/1261#discussion_r671302065



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -5079,6 +5079,38 @@ private void incrementSequenceValues(List<SequenceAllocation> sequenceAllocation
         }
     }
 
+    /**
+     * checks if sequenceAllocation's sequence there in sequenceMap, also returns Global Sequences
+     * from Tenant sequenceAllocations
+     * @param sequenceAllocation
+     * @return
+     */
+
+    private Sequence getSequence(SequenceAllocation sequenceAllocation) {
+        SequenceKey key = sequenceAllocation.getSequenceKey();
+        if (key.getTenantId() == null) {
+            return sequenceMap.putIfAbsent(key, new Sequence(key));
+        } else {
+            Sequence sequence = sequenceMap.get(key);
+            if (sequence == null) {
+                for (Map.Entry<SequenceKey,Sequence> entry : sequenceMap.entrySet()) {
+                    if (compareSequenceKeysWithoutTenant(key, entry.getKey())) {
+                        return entry.getValue();
+                    }
+                }
+            } else {
+                return sequence;
+            }
+        }
+        return null;
+    }
+
+    private boolean compareSequenceKeysWithoutTenant(SequenceKey key1, SequenceKey key2) {
+        return key2.getTenantId() == null && (key1.getSchemaName() == null ? key2.getSchemaName() == null :
+                key1.getSchemaName().equals(key2.getSchemaName())) &&
+                key1.getSequenceName().equals(key2.getSequenceName());

Review comment:
       Here we are checking if key2 is global sequence right?
   
   It's bit difficult to understand in one statement. Good to break it down, something like
   ```
       private boolean compareSequenceKeysWithoutTenant(SequenceKey keyToCompare,
               SequenceKey availableKey) {
           boolean sameSchema = keyToCompare.getSchemaName() == null
                   ? availableKey.getSchemaName() == null
                   : keyToCompare.getSchemaName().equals(availableKey.getSchemaName());
           if (!sameSchema) {
               return false;
           }
           if (availableKey.getTenantId() != null) {
               return false;
           }
           return keyToCompare.getSequenceName().equals(availableKey.getSequenceName());
       }
   ```

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
##########
@@ -5079,6 +5079,38 @@ private void incrementSequenceValues(List<SequenceAllocation> sequenceAllocation
         }
     }
 
+    /**
+     * checks if sequenceAllocation's sequence there in sequenceMap, also returns Global Sequences
+     * from Tenant sequenceAllocations
+     * @param sequenceAllocation
+     * @return
+     */
+
+    private Sequence getSequence(SequenceAllocation sequenceAllocation) {
+        SequenceKey key = sequenceAllocation.getSequenceKey();
+        if (key.getTenantId() == null) {
+            return sequenceMap.putIfAbsent(key, new Sequence(key));
+        } else {
+            Sequence sequence = sequenceMap.get(key);
+            if (sequence == null) {
+                for (Map.Entry<SequenceKey,Sequence> entry : sequenceMap.entrySet()) {
+                    if (compareSequenceKeysWithoutTenant(key, entry.getKey())) {
+                        return entry.getValue();
+                    }
+                }

Review comment:
       nit: replace with stream -> filter -> findfirst?
   ```
   return sequenceMap.entrySet().stream()
           .filter(entry -> compareSequenceKeysWithoutTenant(key, entry.getKey()))
           .findFirst()
           .map(Entry::getValue)
           .orElse(null);
   ```




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