[jira] [Commented] (PHOENIX-1576) Refactor property-related code to reside in ConnectionQueryServicesImpl

["James Taylor (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PHOENIX-1576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14278088#comment-14278088 ] 

James Taylor commented on PHOENIX-1576:
---------------------------------------

Nice improvement, [~samarthjain]! One minor nit:
- Is there a reason to change the loop here for modifyColumnFamilyDescriptor, as the old way doesn't need to do any map lookups? Also, can you just have the one modifyColumnFamilyDescriptor call the other modifyColumnFamilyDescriptor in a loop instead of duplicating the code?
{code}
     private void modifyColumnFamilyDescriptor(HColumnDescriptor hcd, Pair<byte[],Map<String,Object>> family) throws SQLException {
-      for (Entry<String, Object> entry : family.getSecond().entrySet()) {
-        String key = entry.getKey();
-        Object value = entry.getValue();
-        hcd.setValue(key, value == null ? null : value.toString());
-      }
+        Map<String, Object> propNameValue = family.getSecond();
+        if (propNameValue != null) {
+            for (String propName : propNameValue.keySet()) {
+                Object value = propNameValue.get(propName);
+                hcd.setValue(propName, value == null ? null : value.toString());
+            }
+        }
     }
-
+    
+    private void modifyColumnFamilyDescriptor(HColumnDescriptor hcd, Map<String,Object> props) throws SQLException {
+        if (props != null) {
+            for (String propName : props.keySet()) {
+                Object value = props.get(propName);
+                hcd.setValue(propName, value == null ? null : value.toString());
+            }
+        }
+    }
+    
{code}


> Refactor property-related code to reside in ConnectionQueryServicesImpl
> -----------------------------------------------------------------------
>
>                 Key: PHOENIX-1576
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1576
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: James Taylor
>            Assignee: Samarth Jain
>         Attachments: PHOENIX-1576.patch
>
>
> The code in MetaDataClient.addColumn() that deals with teasing apart Phoenix, HColumnDescriptor, and HTableDescriptor properties should all reside in ConnectionQueryServicesImpl, instead of being split between this and MetaDataClient. We should build up an HTableDescriptor with all necessary changes and issue a single modifyTable call instead of making separate RPCs for each change.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)