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

ASF GitHub Bot commented on PHOENIX-6247:
-----------------------------------------

gjacoby126 commented on a change in pull request #1170:
URL: https://github.com/apache/phoenix/pull/1170#discussion_r605997136



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/PTable.java
##########
@@ -728,6 +729,13 @@ private static int getReservedQualifier(byte[] bytes, int 
offset, int length) {
      * (use @getPhysicalTableName for this case) 
      */
     PName getParentTableName();
+
+    /**
+     * @return the logical name of the parent. In case of the view index, it 
is the _IDX_+logical name of base table

Review comment:
       why _IDX_ + logical name of base table for view index? That's the 
physical table the view index is stored in, not the logical name. 

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/PTable.java
##########
@@ -583,6 +583,7 @@ private static int getReservedQualifier(byte[] bytes, int 
offset, int length) {
     PName getName();
     PName getSchemaName();
     PName getTableName();
+    PName getPhysicalTableNameColumnInSyscat();

Review comment:
       Thinking about this more, I don't think just changing the name solves 
the problem of having two methods that are almost-but-not-quite the same thing. 
I'd still be confused about which to call. 
   
   Can we not use the existing getPhysicalNames() to return the physical table 
name column from syscat in the situations where that's appropriate?

##########
File path: phoenix-core/src/main/protobuf/PTable.proto
##########
@@ -111,6 +111,9 @@ message PTable {
   optional bool viewModifiedPhoenixTTL = 44;
   optional int64 lastDDLTimestamp = 45;
   optional bool changeDetectionEnabled = 46;
+  optional bytes physicalTableNameBytes = 47;
+  optional bool isModifiable = 48;

Review comment:
       Since we generate proto code on-demand now, I don't think we gain 
anything by clumping unrelated protobuf changes into this PR. We can save 
isModifiable for next time when the change can be considered as a whole

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/PTable.java
##########
@@ -583,6 +583,7 @@ private static int getReservedQualifier(byte[] bytes, int 
offset, int length) {
     PName getName();
     PName getSchemaName();
     PName getTableName();
+    PName getPhysicalTableNameColumnInSyscat();

Review comment:
       getPhysicalNames makes a point of returning a list of PNames to leave 
the interface open for views that span multiple tables (not currently supported 
but long on the wishlist). Does anything in this PR, such as 
getPhysicalTableNameColumnInSyscat returning a single String, prevent us from 
having multi-table views later?

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/PTable.java
##########
@@ -728,6 +729,13 @@ private static int getReservedQualifier(byte[] bytes, int 
offset, int length) {
      * (use @getPhysicalTableName for this case) 
      */
     PName getParentTableName();
+
+    /**
+     * @return the logical name of the parent. In case of the view index, it 
is the _IDX_+logical name of base table
+     * Ex: For hierarchical views like tableLogicalName --> view1 --> view2, 
for view2, returns _IDX_+tableLogicalName
+     */
+    PName getParentLogicalName();

Review comment:
       Likewise with getParentName vs getParentLogicalName. If I'm reading the 
current logic right, getParentName _also_ returns only logical names. They're 
almost-but-not-quite the same thing. Can we consolidate? Otherwise we're going 
to create lots of subtle, really-hard-to-spot bugs going forward when people 
use the wrong one. 




-- 
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.

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


> Change SYSTEM.CATALOG to allow separation of physical name (Hbase name) from 
> logical name (Phoenix name)
> --------------------------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-6247
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-6247
>             Project: Phoenix
>          Issue Type: Improvement
>            Reporter: Gokcen Iskender
>            Assignee: Gokcen Iskender
>            Priority: Major
>
> Currently, the tables in Phoenix have the same name as the underlying Hbase 
> table. Separating logical and physical table name, ie. Having a Phoenix table 
> point to an Hbase table with a different name have some advantages. 
> An example is this: Let's say we want to have a different storage/encoding 
> scheme for an index. We can build the new index while the clients use the old 
> index and once the index is rebuilt, we can momentarily start pointing to the 
> new index table without much downtime or performance implications. For the 
> client, they are using the same index with the same name, but the physical 
> table is different. Today, in order to change the index like this, we have to 
> drop it and re-create which is a downtime for the index and the data table 
> full scans are used for queries impacting performance while the index 
> creation goes on.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to