[jira] [Commented] (PHOENIX-3002) Upgrading to 4.8 doesn't recreate local indexes

[Samarth Jain (JIRA)]

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

Samarth Jain commented on PHOENIX-3002:
---------------------------------------

Thanks for the patch, [~chrajeshbab...@gmail.com]. Here is some minor feedback:

Please remove the comment here or replace with something more appropriate to 
local indexes
{code}
+        // don't use the passed timestamp as scn because we want to query all 
view indexes up to
+        // now.
+        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB,
+            Long.toString(HConstants.LATEST_TIMESTAMP));
{code}

{code}
public static String getColumnDisplayName(String cf, String cq) {
        return getName(cf == null || cf.isEmpty() ? null : cf, cq, false);
    }
    
    public static String getCaseSensitiveColumnDisplayName(String cf, String 
cq) {
        return getName(cf == null || cf.isEmpty() ? null : cf, cq, true);
    }
{code}

I don't think your patch compiles. The method signature doesn't accept boolean 
as second param.
{code}
+                                    metaConnection = 
upgradeLocalIndexes(metaConnection, true);
{code}

I think you can use these existing utility method in SchemaUtil. If you would 
like, maybe you can rename them to something like getColumnName()?  

No need to qualify with class name here.
{code}
+            sqlEx = 
ConnectionQueryServicesImpl.closeConnection(metaConnection, sqlEx);
+            sqlEx = 
ConnectionQueryServicesImpl.closeConnection(globalConnection, sqlEx);
{code}

You can just do getAdmin() here. Also, please include this call in a try with 
resources construct. You won't need to close the admin explicitly then in the 
finally block.
{code}
+            admin = globalConnection.getQueryServices().getAdmin();
{code}


Please leave the method as private as I don't see uses of it outside of this 
class:
{code}
-    private static SQLException closeConnection(PhoenixConnection conn, 
SQLException sqlEx) {
+    public static SQLException closeConnection(PhoenixConnection conn, 
SQLException sqlEx) {
{code}

In MetadataClient:
{code}
private void deleteFromStatsTable(List<TableRef> tableRefs, long ts) throws 
SQLException {
         Properties props = new Properties(connection.getClientInfo());
         props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, 
Long.toString(ts));
-        Connection conn = DriverManager.getConnection(connection.getURL(), 
props);
+        Connection conn = new PhoenixConnection(connection.getQueryServices(), 
connection, ts);
         conn.setAutoCommit(true);
{code}
Why is this change needed? My guess is earlier the upgrade was getting 
triggered because of this code (getting connection through DriverManager will 
call ConnectionQueryServicesImpl.init()). If this indeed is the case then maybe 
you don't need to move the code out of UpgradeUtil? 





> Upgrading to 4.8 doesn't recreate local indexes
> -----------------------------------------------
>
>                 Key: PHOENIX-3002
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-3002
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: Samarth Jain
>            Assignee: Rajeshbabu Chintaguntla
>            Priority: Blocker
>             Fix For: 4.8.0
>
>         Attachments: PHOENIX-3002.patch, PHOENIX-3002_v0.patch
>
>
> [~rajeshbabu] - I noticed that when upgrading to 4.8, local indexes created 
> with 4.7 or before aren't getting recreated with the new local indexes 
> implementation.  I am not seeing the metadata rows for the recreated indices 
> in SYSTEM.CATALOG.



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