phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From JamesRTaylor <...@git.apache.org>
Subject [GitHub] phoenix pull request #295: PHOENIX-4579: Add a config to conditionally creat...
Date Wed, 04 Apr 2018 00:41:44 GMT
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.


---

Mime
View raw message