Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/295#discussion_r179000414
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
 ---
    @@ -2643,6 +2661,26 @@ public void upgradeSystemTables(final String url, 
final Properties props) throws
                 metaConnection.setRunningUpgrade(true);
                 try {
                     
metaConnection.createStatement().executeUpdate(QueryConstants.CREATE_TABLE_METADATA);
    +
    +                // HBase Namespace SYSTEM is created by {@link 
ensureSystemTablesMigratedToSystemNamespace(ReadOnlyProps)} method
    +                // This statement will create its entry in SYSCAT table, 
so that GRANT/REVOKE commands can work
    +                // with SYSTEM Namespace. (See PHOENIX-4227 
https://issues.apache.org/jira/browse/PHOENIX-4227)
    +                if (SchemaUtil.isNamespaceMappingEnabled(PTableType.SYSTEM,
    +                  ConnectionQueryServicesImpl.this.getProps())) {
    --- End diff --
    
    If autoUpgrade is enabled, we don't want the user to have to run EXECUTE 
UPGRADE. Move the creation of the namespace to ensureTableCreated if necessary 
(but keep the ensureSystemTablesMigratedToSystemNamespace logic in 
upgradeSystemTables). Let's have a test for this too.
    
    I think it'd be better to have the acquireUpgradeMutex only in 
upgradeSystemTables so we only do this once. We can acquire it before calling 
ensureSystemTablesMigratedToSystemNamespace.


---

Reply via email to