stoty commented on PR #1480:
URL: https://github.com/apache/phoenix/pull/1480#issuecomment-1212187121

   (We've had offline discussions about this with Richard, so some context may 
not be immediately apparent from the patch)
   
   You have uncovered two different additional problems here.
   
   One is that moveChildLinkConnection doesn't handle namespace mapping.
   Your current solution here is incorrect, because it manipulates the 
pre-upgrade default namespace system tables, even though by this point it has 
already been moved to the NS (and upgraded).
   The correct solution would be to replace both  
_TableName.valueOf(SYSTEM_CATALOG_NAME)_ and 
TableName.valueOf(SYSTEM_CHILD_LINK_NAME) with the right namespace aware method 
from SchmaUtil which preforms the '.' -> ':' replacement as needed.  (it is 
also much simpler)
   
   This bugfix may even be split into a separate dependent ticket, if you 
prefer.
   We should also check if we have other similar bugs in the upgrade code.
   
   The other problem is that moveChildLinkConnection creates a the HBase 
Connection from a vanilla Configuration object (reading from hbase-site.xml), 
which doesn't get necessary Config changes that were set by 
BaseTest/minicluster.
   
   Simply copying the ZK quorum from the CQSI connection may get the test to 
pass, but is not the right way.
   We should clone the Configuration object from CQSI,  apply the necessary  
the changes to the clone, and use that for creating the HBaseConnection.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to