[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-31 Thread m2je 
Github user m2je commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r206717705
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java ---
@@ -1223,7 +1225,9 @@ public void readFields(DataInput input) throws 
IOException {
 boolean hasViewIndexId = encodedIndexedColumnsAndViewId < 0;
 if (hasViewIndexId) {
 // Fixed length
-viewIndexId = new 
byte[MetaDataUtil.getViewIndexIdDataType().getByteSize()];
+//Use leacy viewIndexIdType for clients older than 4.10 release
--- End diff --

done


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-31 Thread m2je 
Github user m2je commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r206715551
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java ---
@@ -204,14 +205,19 @@ private void appendPKColumnValue(StringBuilder buf, 
byte[] range, Boolean isNull
 range = ptr.get();
 }
 if (changeViewIndexId) {
-Short s = (Short) type.toObject(range);
-s = (short) (s + (-Short.MAX_VALUE));
-buf.append(s.toString());
+PDataType viewIndexDataType = 
tableRef.getTable().getViewIndexType();
+buf.append(getViewIndexValue(type, range, 
viewIndexDataType).toString());
 } else {
 Format formatter = context.getConnection().getFormatter(type);
 buf.append(type.toStringLiteral(range, formatter));
 }
 }
+
+private Long getViewIndexValue(PDataType type, byte[] range, PDataType 
viewIndexDataType){
+boolean useLongViewIndex = 
MetaDataUtil.getViewIndexIdDataType().equals(viewIndexDataType);
+Object s =  type.toObject(range);
+return (useLongViewIndex ? (Long) s : (Short) s) - 
(useLongViewIndex ? Long.MAX_VALUE : Short.MAX_VALUE);
--- End diff --

Done


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-31 Thread m2je 
Github user m2je commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r206715609
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
 ---
@@ -1431,11 +1438,42 @@ private PTable getTable(RegionScanner scanner, long 
clientTimeStamp, long tableT
 // server while holding this lock is a bad idea and likely to 
cause contention.
 return PTableImpl.makePTable(tenantId, schemaName, tableName, 
tableType, indexState, timeStamp, tableSeqNum,
 pkName, saltBucketNum, columns, parentSchemaName, 
parentTableName, indexes, isImmutableRows, physicalTables, defaultFamilyName,
-viewStatement, disableWAL, multiTenant, storeNulls, 
viewType, viewIndexId, indexType,
+viewStatement, disableWAL, multiTenant, storeNulls, 
viewType, viewIndexType, viewIndexId, indexType,
 rowKeyOrderOptimizable, transactionProvider, 
updateCacheFrequency, baseColumnCount,
 indexDisableTimestamp, isNamespaceMapped, 
autoPartitionSeq, isAppendOnlySchema, storageScheme, encodingScheme, cqCounter, 
useStatsForParallelization);
 }
+private Long getViewIndexId(Cell[] tableKeyValues, PDataType 
viewIndexType) {
+Cell viewIndexIdKv = tableKeyValues[VIEW_INDEX_ID_INDEX];
+return viewIndexIdKv == null ? null :
+decodeViewIndexId(viewIndexIdKv, viewIndexType);
+}
 
+/**
+ * check the value for {@value USE_LONG_VIEW_INDEX} and if its present 
consider viewIndexId as long otherwise
+ * read as short and convert it to long
+ *
+ * @param tableKeyValues
+ * @param viewIndexType
+ * @return
+ */
+private Long decodeViewIndexId(Cell viewIndexIdKv,  PDataType 
viewIndexType) {
+boolean useLongViewIndex = 
MetaDataUtil.getViewIndexIdDataType().equals(viewIndexType);
+   return new Long(
+   useLongViewIndex
+   ? 
viewIndexType.getCodec().decodeLong(viewIndexIdKv.getValueArray(),
+   viewIndexIdKv.getValueOffset(), 
SortOrder.getDefault())
+   : 
MetaDataUtil.getLegacyViewIndexIdDataType().getCodec().decodeShort(viewIndexIdKv.getValueArray(),
+   viewIndexIdKv.getValueOffset(), 
SortOrder.getDefault())
+   );
+}
+
+private PDataType getViewIndexType(Cell[] tableKeyValues) {
+Cell useLongViewIndexKv = tableKeyValues[USE_LONG_VIEW_INDEX];
--- End diff --

done


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-31 Thread m2je 
Github user m2je commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r206715585
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
 ---
@@ -2185,7 +2223,7 @@ public void createTable(RpcController controller, 
CreateTableRequest request,
 throw sqlExceptions[0];
 }
 long seqValue = seqValues[0];
-if (seqValue > Short.MAX_VALUE) {
+if (seqValue > Long.MAX_VALUE) {
--- End diff --

done


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-30 Thread JamesRTaylor 
Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r206328431
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java ---
@@ -204,14 +205,19 @@ private void appendPKColumnValue(StringBuilder buf, 
byte[] range, Boolean isNull
 range = ptr.get();
 }
 if (changeViewIndexId) {
-Short s = (Short) type.toObject(range);
-s = (short) (s + (-Short.MAX_VALUE));
-buf.append(s.toString());
+PDataType viewIndexDataType = 
tableRef.getTable().getViewIndexType();
+buf.append(getViewIndexValue(type, range, 
viewIndexDataType).toString());
 } else {
 Format formatter = context.getConnection().getFormatter(type);
 buf.append(type.toStringLiteral(range, formatter));
 }
 }
+
+private Long getViewIndexValue(PDataType type, byte[] range, PDataType 
viewIndexDataType){
+boolean useLongViewIndex = 
MetaDataUtil.getViewIndexIdDataType().equals(viewIndexDataType);
+Object s =  type.toObject(range);
+return (useLongViewIndex ? (Long) s : (Short) s) - 
(useLongViewIndex ? Long.MAX_VALUE : Short.MAX_VALUE);
--- End diff --

Shouldn't the  argument be all that's necessary? Why is an additional 
type argument needed? The type as defined in the schema should be the right 
type, no?


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-30 Thread JamesRTaylor 
Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r206326382
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
 ---
@@ -1431,11 +1438,42 @@ private PTable getTable(RegionScanner scanner, long 
clientTimeStamp, long tableT
 // server while holding this lock is a bad idea and likely to 
cause contention.
 return PTableImpl.makePTable(tenantId, schemaName, tableName, 
tableType, indexState, timeStamp, tableSeqNum,
 pkName, saltBucketNum, columns, parentSchemaName, 
parentTableName, indexes, isImmutableRows, physicalTables, defaultFamilyName,
-viewStatement, disableWAL, multiTenant, storeNulls, 
viewType, viewIndexId, indexType,
+viewStatement, disableWAL, multiTenant, storeNulls, 
viewType, viewIndexType, viewIndexId, indexType,
 rowKeyOrderOptimizable, transactionProvider, 
updateCacheFrequency, baseColumnCount,
 indexDisableTimestamp, isNamespaceMapped, 
autoPartitionSeq, isAppendOnlySchema, storageScheme, encodingScheme, cqCounter, 
useStatsForParallelization);
 }
+private Long getViewIndexId(Cell[] tableKeyValues, PDataType 
viewIndexType) {
+Cell viewIndexIdKv = tableKeyValues[VIEW_INDEX_ID_INDEX];
+return viewIndexIdKv == null ? null :
+decodeViewIndexId(viewIndexIdKv, viewIndexType);
+}
 
+/**
+ * check the value for {@value USE_LONG_VIEW_INDEX} and if its present 
consider viewIndexId as long otherwise
+ * read as short and convert it to long
+ *
+ * @param tableKeyValues
+ * @param viewIndexType
+ * @return
+ */
+private Long decodeViewIndexId(Cell viewIndexIdKv,  PDataType 
viewIndexType) {
+boolean useLongViewIndex = 
MetaDataUtil.getViewIndexIdDataType().equals(viewIndexType);
+   return new Long(
+   useLongViewIndex
+   ? 
viewIndexType.getCodec().decodeLong(viewIndexIdKv.getValueArray(),
+   viewIndexIdKv.getValueOffset(), 
SortOrder.getDefault())
+   : 
MetaDataUtil.getLegacyViewIndexIdDataType().getCodec().decodeShort(viewIndexIdKv.getValueArray(),
+   viewIndexIdKv.getValueOffset(), 
SortOrder.getDefault())
+   );
+}
+
+private PDataType getViewIndexType(Cell[] tableKeyValues) {
+Cell useLongViewIndexKv = tableKeyValues[USE_LONG_VIEW_INDEX];
--- End diff --

I think we need to keep the single VIEW_INDEX_ID column and make sure it's 
type is defined (through a property) at create time (and not allow it to be 
changed). The issue isn't with the metadata, but with the row key of the rows 
of the table. In old tables, it'll be a short while for new tables it'll be a 
long. We don't want to have to rewrite the data.


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-28 Thread twdsilva 
Github user twdsilva commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r205953961
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java ---
@@ -204,14 +205,19 @@ private void appendPKColumnValue(StringBuilder buf, 
byte[] range, Boolean isNull
 range = ptr.get();
 }
 if (changeViewIndexId) {
-Short s = (Short) type.toObject(range);
-s = (short) (s + (-Short.MAX_VALUE));
-buf.append(s.toString());
+PDataType viewIndexDataType = 
tableRef.getTable().getViewIndexType();
+buf.append(getViewIndexValue(type, range, 
viewIndexDataType).toString());
 } else {
 Format formatter = context.getConnection().getFormatter(type);
 buf.append(type.toStringLiteral(range, formatter));
 }
 }
+
+private Long getViewIndexValue(PDataType type, byte[] range, PDataType 
viewIndexDataType){
+boolean useLongViewIndex = 
MetaDataUtil.getViewIndexIdDataType().equals(viewIndexDataType);
+Object s =  type.toObject(range);
+return (useLongViewIndex ? (Long) s : (Short) s) - 
(useLongViewIndex ? Long.MAX_VALUE : Short.MAX_VALUE);
--- End diff --

 @JamesRTaylor  or @chrajeshbabu do you know if this change is correct?


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-28 Thread twdsilva 
Github user twdsilva commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r205954264
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
 ---
@@ -1431,11 +1438,42 @@ private PTable getTable(RegionScanner scanner, long 
clientTimeStamp, long tableT
 // server while holding this lock is a bad idea and likely to 
cause contention.
 return PTableImpl.makePTable(tenantId, schemaName, tableName, 
tableType, indexState, timeStamp, tableSeqNum,
 pkName, saltBucketNum, columns, parentSchemaName, 
parentTableName, indexes, isImmutableRows, physicalTables, defaultFamilyName,
-viewStatement, disableWAL, multiTenant, storeNulls, 
viewType, viewIndexId, indexType,
+viewStatement, disableWAL, multiTenant, storeNulls, 
viewType, viewIndexType, viewIndexId, indexType,
 rowKeyOrderOptimizable, transactionProvider, 
updateCacheFrequency, baseColumnCount,
 indexDisableTimestamp, isNamespaceMapped, 
autoPartitionSeq, isAppendOnlySchema, storageScheme, encodingScheme, cqCounter, 
useStatsForParallelization);
 }
+private Long getViewIndexId(Cell[] tableKeyValues, PDataType 
viewIndexType) {
+Cell viewIndexIdKv = tableKeyValues[VIEW_INDEX_ID_INDEX];
+return viewIndexIdKv == null ? null :
+decodeViewIndexId(viewIndexIdKv, viewIndexType);
+}
 
+/**
+ * check the value for {@value USE_LONG_VIEW_INDEX} and if its present 
consider viewIndexId as long otherwise
+ * read as short and convert it to long
+ *
+ * @param tableKeyValues
+ * @param viewIndexType
+ * @return
+ */
+private Long decodeViewIndexId(Cell viewIndexIdKv,  PDataType 
viewIndexType) {
+boolean useLongViewIndex = 
MetaDataUtil.getViewIndexIdDataType().equals(viewIndexType);
+   return new Long(
+   useLongViewIndex
+   ? 
viewIndexType.getCodec().decodeLong(viewIndexIdKv.getValueArray(),
+   viewIndexIdKv.getValueOffset(), 
SortOrder.getDefault())
+   : 
MetaDataUtil.getLegacyViewIndexIdDataType().getCodec().decodeShort(viewIndexIdKv.getValueArray(),
+   viewIndexIdKv.getValueOffset(), 
SortOrder.getDefault())
+   );
+}
+
+private PDataType getViewIndexType(Cell[] tableKeyValues) {
+Cell useLongViewIndexKv = tableKeyValues[USE_LONG_VIEW_INDEX];
--- End diff --

For existing tables the VIEW_INDEX_ID  column will store the index as a 
short while for new tables the value will be stored as a long. 
Would it be cleaner to create a new column that stores the view index id as 
a long. In QueryServicesConnectionImpl.upgradeSystemTables()  we would set the 
new column based on the existing value. Finally we can remove the existing 
VIEW_INDEX_ID in the next release. 
WDYT @JamesRTaylor ?






---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-28 Thread twdsilva 
Github user twdsilva commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r205952155
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java ---
@@ -1223,7 +1225,9 @@ public void readFields(DataInput input) throws 
IOException {
 boolean hasViewIndexId = encodedIndexedColumnsAndViewId < 0;
 if (hasViewIndexId) {
 // Fixed length
-viewIndexId = new 
byte[MetaDataUtil.getViewIndexIdDataType().getByteSize()];
+//Use leacy viewIndexIdType for clients older than 4.10 release
--- End diff --

nit: typo 


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-28 Thread twdsilva 
Github user twdsilva commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/317#discussion_r205952450
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
 ---
@@ -2185,7 +2223,7 @@ public void createTable(RpcController controller, 
CreateTableRequest request,
 throw sqlExceptions[0];
 }
 long seqValue = seqValues[0];
-if (seqValue > Short.MAX_VALUE) {
+if (seqValue > Long.MAX_VALUE) {
--- End diff --

This check is no longer needed since the indexId is now a Long. When you 
call incrementSequences if the current sequence value is LONG.MAX_VALUE you 
will get an exception.


---

[GitHub] phoenix pull request #317: PHOENIX-3547 Supporting more number of indices pe...

2018-07-28 Thread m2je 
GitHub user m2je opened a pull request:

https://github.com/apache/phoenix/pull/317

PHOENIX-3547 Supporting more number of indices per table.

Currently the number of indices per Phoenix table is bound to maximum of 
65535 (java.lang.Short) which is a limitation for applications requiring to 
have unlimited number of indices.
This change will consider any new table created in Phoenix to support view 
index ids to be in the range of -9,223,372,036,854,775,808 to 
9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to 
cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 
65535 of indices.
A new boolean column (USE_LONG_VIEW_INDEX BOOLEAN DEFAULT FALSE) is added 
to SYSTEM.CATALOG to specify each Phoenix table's support for large number of 
indices.
On each new Phoenix table creation the value for USE_LONG_VIEW_INDEX will 
be set to `true` while this value would be false for any existing table.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/m2je/phoenix PHOENIX-3547

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/phoenix/pull/317.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #317


commit f1ffcf6b76a0a764df5529a03ba8dc42112dc5a9
Author: Mahdi Salarkia 
Date:   2018-07-28T05:27:46Z

PHOENIX-3547 Supporting more number of indices per table.

Currently the number of indices per Phoenix table is bound to maximum of 
65535 (java.lang.Short) which is a limitation for applications requiring to 
have unlimited number of indices.
This change will consider any new table created in Phoenix to support view 
index ids to be in the range of -9,223,372,036,854,775,808 to 
9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to 
cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 
65535 of indices.
A new boolean column (USE_LONG_VIEW_INDEX BOOLEAN DEFAULT FALSE) is added 
to SYSTEM.CATALOG to specify each Phoenix table's support for large number of 
indices.
On each new Phoenix table creation the value for USE_LONG_VIEW_INDEX will 
be set to `true` while this value would be false for any existing table.




---