[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r64124279
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
 ---
@@ -165,6 +165,9 @@ public RegionScanner 
preScannerOpen(ObserverContext

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r64124199
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java ---
@@ -292,6 +294,119 @@ private static void 
preSplitSequenceTable(PhoenixConnection conn, int nSaltBucke
 }
 }
 }
+
+public static void upgradeLocalIndexes(PhoenixConnection 
metaConnection) throws SQLException,
+IOException, org.apache.hadoop.hbase.TableNotFoundException {
+HBaseAdmin admin = null;
+try {
+admin = metaConnection.getQueryServices().getAdmin();
+ResultSet rs = 
metaConnection.createStatement().executeQuery("SELECT TABLE_SCHEM, TABLE_NAME, 
DATA_TABLE_NAME FROM SYSTEM.CATALOG  "
++ "  WHERE COLUMN_NAME IS NULL"
++ "   AND COLUMN_FAMILY IS NULL"
++ "   AND INDEX_TYPE=2");
+boolean droppedLocalIndexes = false;
+while (rs.next()) {
+if(!droppedLocalIndexes) {
+HTableDescriptor[] localIndexTables = 
admin.listTables(MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX+".*");
+String localIndexSplitter = 
LocalIndexSplitter.class.getName();
+for (HTableDescriptor table : localIndexTables) {
+HTableDescriptor dataTableDesc = 
admin.getTableDescriptor(TableName.valueOf(MetaDataUtil.getUserTableName(table.getNameAsString(;
+HColumnDescriptor[] columnFamilies = 
dataTableDesc.getColumnFamilies();
+boolean modifyTable = false;
+for(HColumnDescriptor cf : columnFamilies) {
+String localIndexCf = 
QueryConstants.LOCAL_INDEX_COLUMN_FAMILY_PREFIX+cf.getNameAsString();
+
if(dataTableDesc.getFamily(Bytes.toBytes(localIndexCf))==null){
+HColumnDescriptor colDef =
+new 
HColumnDescriptor(localIndexCf);
+for(EntrykeyValue: cf.getValues().entrySet()){
+
colDef.setValue(keyValue.getKey().copyBytes(), keyValue.getValue().copyBytes());
+}
+dataTableDesc.addFamily(colDef);
+modifyTable = true;
+}
+}
+List coprocessors = 
dataTableDesc.getCoprocessors();
+for(String coprocessor:  coprocessors) {
+if(coprocessor.equals(localIndexSplitter)) {
+
dataTableDesc.removeCoprocessor(localIndexSplitter);
+modifyTable = true;
+}
+}
+if(modifyTable) {
+admin.modifyTable(dataTableDesc.getName(), 
dataTableDesc);
+}
+}
+
admin.disableTables(MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX+".*");
+
admin.deleteTables(MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX+".*");
+droppedLocalIndexes = true;
+}
+String getColumns =
+"SELECT COLUMN_NAME, COLUMN_FAMILY FROM 
SYSTEM.CATALOG  WHERE TABLE_SCHEM "
++ (rs.getString(1) == null ? "IS NULL " : 
"='" + rs.getString(1)
++ "'") + " and TABLE_NAME='" + 
rs.getString(2)
++ "' AND COLUMN_NAME IS NOT NULL";
+ResultSet getColumnsRs = 
metaConnection.createStatement().executeQuery(getColumns);
+List indexedColumns = new ArrayList(1);
+List coveredColumns = new ArrayList(1);
+
+while (getColumnsRs.next()) {
+String column = getColumnsRs.getString(1);
+String columnName = 
IndexUtil.getDataColumnName(column);
+if 
(columnName.equals(MetaDataUtil.getViewIndexIdColumnName())) {
+continue;
+}
+String columnFamily = 
IndexUtil.getDataColumnFamilyName(column);
+if (getColumnsRs.getString(2) == null) {
+if (columnFamily != null && 
!columnFamily.isEmpty()) {
+if 
(columnFamily.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)) {
+

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r64123714
  
--- Diff: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinIT.java ---
@@ -187,13 +186,13 @@ public void initTable() throws Exception {
 "CREATE LOCAL INDEX \"idx_supplier\" ON " + 
JOIN_SUPPLIER_TABLE_FULL_NAME + " (name)"
 }, {
 "SORT-MERGE-JOIN (LEFT) TABLES\n" +
-"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + 
MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX + JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " 
[-32768]\n" +
+"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " 
+JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " [1]\n" +
--- End diff --

I will work on this in other patch James. Again I need to change all the 
test cases.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r64123637
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
 ---
@@ -2455,6 +2409,19 @@ public Void call() throws Exception {
 }
 
 if (currentServerSideTableTimeStamp < 
MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_8_0) {
+Properties props = 
PropertiesUtil.deepCopy(metaConnection.getClientInfo());
+
props.remove(PhoenixRuntime.CURRENT_SCN_ATTRIB);
+
props.remove(PhoenixRuntime.TENANT_ID_ATTRIB);
+PhoenixConnection conn =
+new 
PhoenixConnection(ConnectionQueryServicesImpl.this,
+
metaConnection.getURL(), props, metaConnection
+
.getMetaDataCache());
+try {
+
UpgradeUtil.upgradeLocalIndexes(conn);
--- End diff --

Agree with you James. Will do it in other patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on the pull request:

https://github.com/apache/phoenix/pull/168#issuecomment-219630101
  
Looks very good, @chrajeshbabu! Thanks so much for this. Are you ok making 
the changes I mentioned?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63469708
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
 ---
@@ -2455,6 +2409,19 @@ public Void call() throws Exception {
 }
 
 if (currentServerSideTableTimeStamp < 
MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_8_0) {
+Properties props = 
PropertiesUtil.deepCopy(metaConnection.getClientInfo());
+
props.remove(PhoenixRuntime.CURRENT_SCN_ATTRIB);
+
props.remove(PhoenixRuntime.TENANT_ID_ATTRIB);
+PhoenixConnection conn =
+new 
PhoenixConnection(ConnectionQueryServicesImpl.this,
+
metaConnection.getURL(), props, metaConnection
+
.getMetaDataCache());
+try {
+
UpgradeUtil.upgradeLocalIndexes(conn);
--- End diff --

Should we only disable all local indexes here as a full rebuild would take 
a lot of time? Then we can let users use your utility to rebuild them?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63469452
  
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/IndexUtil.java 
---
@@ -667,4 +651,28 @@ public static String 
getIndexColumnExpressionStr(PColumn col) {
 return col.getExpressionStr() == null ? 
IndexUtil.getCaseSensitiveDataColumnFullName(col.getName().getString())
 : col.getExpressionStr();
 }
+
+public static void 
addLocalUpdatesToCpOperations(ObserverContext c,
--- End diff --

Minor nit, but if this is called in only one place, can we move this there 
and make it private?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63469161
  
--- Diff: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java 
---
@@ -707,16 +702,17 @@ public void testIndexHalfStoreFileReader() throws 
Exception {
 } catch (Exception ex) {
   Log.info(ex);
 }
-
-long waitStartTime = System.currentTimeMillis();
-// wait until merge happened
-while (System.currentTimeMillis() - waitStartTime < 1) 
{
-  List regions = 
admin.getTableRegions(indexTable);
-  System.out.println("Waiting:" + regions.size());
-  if (regions.size() < numRegions) {
-break;
-  }
-  Threads.sleep(1000);
+if(!localIndex) {
+long waitStartTime = System.currentTimeMillis();
+// wait until merge happened
+while (System.currentTimeMillis() - waitStartTime < 
1) {
+  List regions = 
admin.getTableRegions(indexTable);
+  System.out.println("Waiting:" + regions.size());
--- End diff --

Remove System.out.println() or replace with logging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63468900
  
--- Diff: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinIT.java ---
@@ -187,13 +186,13 @@ public void initTable() throws Exception {
 "CREATE LOCAL INDEX \"idx_supplier\" ON " + 
JOIN_SUPPLIER_TABLE_FULL_NAME + " (name)"
 }, {
 "SORT-MERGE-JOIN (LEFT) TABLES\n" +
-"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + 
MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX + JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " 
[-32768]\n" +
+"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " 
+JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " [1]\n" +
--- End diff --

Yes, I think it'd be good to mention the local index name as there could be 
more than one. If you include the full name of the local index (. which is what you get from PTable.getName()), then that's 
unique so you wouldn't need to include the data table name too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on the pull request:

https://github.com/apache/phoenix/pull/168#issuecomment-219617896
  
James as we discussed I have made a patch working with older versions of 
HBase first and handled review comments here. Will create new pull request with 
that patch.

Thanks for reviews.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463623
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixTransactionalIndexer.java
 ---
@@ -160,6 +163,9 @@ public void 
preBatchMutate(ObserverContext c,
 
 // get the index updates for all elements in this batch
 indexUpdates = getIndexUpdates(c.getEnvironment(), 
indexMetaData, getMutationIterator(miniBatchOp), txRollbackAttribute);
+
+IndexUtil.addLocalUpdatesToCpOperations(c, miniBatchOp, 
indexUpdates,
+m.getDurability() != Durability.SKIP_WAL);
--- End diff --

Yes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463618
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/hbase/index/write/ParallelWriterIndexCommitter.java
 ---
@@ -116,7 +117,10 @@ public void write(Multimap toWrite) throws S
 // doing a complete copy over of all the index update for each 
table.
 final List mutations = 
kvBuilder.cloneIfNecessary((List)entry.getValue());
 final HTableInterfaceReference tableReference = entry.getKey();
-final RegionCoprocessorEnvironment env = this.env;
+if (env !=null && tableReference.getTableName().equals(
+env.getRegion().getTableDesc().getNameAsString())) {
+continue;
+}
--- End diff --

Yes we should do that. Currently made a patch without HBASE-15600 so once 
it's ready then will add condition check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463584
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java ---
@@ -1016,7 +1016,7 @@ private MutationState buildIndexAtTimeStamp(PTable 
index, NamedTableNode dataTab
 // connection so that our new index table is visible.
 Properties props = new Properties(connection.getClientInfo());
 props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, 
Long.toString(connection.getSCN()+1));
-PhoenixConnection conn = 
DriverManager.getConnection(connection.getURL(), 
props).unwrap(PhoenixConnection.class);
+PhoenixConnection conn = new PhoenixConnection(connection, 
connection.getQueryServices(), props);
--- End diff --

DriverManager.getConnection has more overhead than initializing the 
PhoenixConnection right that's why changed. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463462
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/execute/DelegateHTable.java ---
@@ -297,4 +297,28 @@ public boolean checkAndDelete(byte[] row, byte[] 
family, byte[] qualifier,
return delegate.checkAndDelete(row, family, qualifier, 
compareOp, value, delete);
}
 
+@Override
--- End diff --

These methods required for 1.2.2-SNAPSHOT version so currently not required 
with older versions. will remove it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463482
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java ---
@@ -861,7 +871,12 @@ public Put buildUpdateMutation(KeyValueBuilder 
kvBuilder, ValueGetter valueGette
 put.setDurability(!indexWALDisabled ? 
Durability.USE_DEFAULT : Durability.SKIP_WAL);
 }
 //this is a little bit of extra work for installations 
that are running <0.94.14, but that should be rare and is a short-term set of 
wrappers - it shouldn't kill GC
-put.add(kvBuilder.buildPut(rowKey, 
ref.getFamilyWritable(), cq, ts, value));
+if(this.isLocalIndex) {
+ColumnReference columnReference = 
this.coveredColumnsMap.get(ref);
+   put.add(kvBuilder.buildPut(rowKey, 
columnReference.getFamilyWritable(), cq, ts, value));
--- End diff --

Yes James. It's to save regenerating the column family name all the time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463350
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
 ---
@@ -202,7 +207,10 @@ protected RegionScanner doPostScannerOpen(final 
ObserverContext

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

Github user chrajeshbabu commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r63463302
  
--- Diff: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinIT.java ---
@@ -187,13 +186,13 @@ public void initTable() throws Exception {
 "CREATE LOCAL INDEX \"idx_supplier\" ON " + 
JOIN_SUPPLIER_TABLE_FULL_NAME + " (name)"
 }, {
 "SORT-MERGE-JOIN (LEFT) TABLES\n" +
-"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + 
MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX + JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " 
[-32768]\n" +
+"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " 
+JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " [1]\n" +
--- End diff --

@JamesRTaylor  Yes agree.  Is this fine "OVER LOCAL INDEX OF DATA_TABLE"? 
or can we include index table name as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62710798
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/hbase/index/write/ParallelWriterIndexCommitter.java
 ---
@@ -116,7 +117,10 @@ public void write(Multimap toWrite) throws S
 // doing a complete copy over of all the index update for each 
table.
 final List mutations = 
kvBuilder.cloneIfNecessary((List)entry.getValue());
 final HTableInterfaceReference tableReference = entry.getKey();
-final RegionCoprocessorEnvironment env = this.env;
+if (env !=null && tableReference.getTableName().equals(
+env.getRegion().getTableDesc().getNameAsString())) {
+continue;
+}
--- End diff --

Can we conditionally *not* filter our local index rows depending on the 
HBase version?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62710902
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixTransactionalIndexer.java
 ---
@@ -160,6 +163,9 @@ public void 
preBatchMutate(ObserverContext c,
 
 // get the index updates for all elements in this batch
 indexUpdates = getIndexUpdates(c.getEnvironment(), 
indexMetaData, getMutationIterator(miniBatchOp), txRollbackAttribute);
+
+IndexUtil.addLocalUpdatesToCpOperations(c, miniBatchOp, 
indexUpdates,
+m.getDurability() != Durability.SKIP_WAL);
--- End diff --

Can we conditionally *not* run this code depending on the HBase version?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62700523
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java ---
@@ -1016,7 +1016,7 @@ private MutationState buildIndexAtTimeStamp(PTable 
index, NamedTableNode dataTab
 // connection so that our new index table is visible.
 Properties props = new Properties(connection.getClientInfo());
 props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, 
Long.toString(connection.getSCN()+1));
-PhoenixConnection conn = 
DriverManager.getConnection(connection.getURL(), 
props).unwrap(PhoenixConnection.class);
+PhoenixConnection conn = new PhoenixConnection(connection, 
connection.getQueryServices(), props);
--- End diff --

Just curious on this change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62699657
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/ExplainTable.java ---
@@ -198,8 +198,14 @@ private void appendPKColumnValue(StringBuilder buf, 
byte[] range, Boolean isNull
 type.coerceBytes(ptr, type, sortOrder, SortOrder.getDefault());
 range = ptr.get();
 }
-Format formatter = context.getConnection().getFormatter(type);
-buf.append(type.toStringLiteral(range, formatter));
+if (changeViewIndexId) {
--- End diff --

I like this trick. We should always do this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62699243
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java ---
@@ -861,7 +871,12 @@ public Put buildUpdateMutation(KeyValueBuilder 
kvBuilder, ValueGetter valueGette
 put.setDurability(!indexWALDisabled ? 
Durability.USE_DEFAULT : Durability.SKIP_WAL);
 }
 //this is a little bit of extra work for installations 
that are running <0.94.14, but that should be rare and is a short-term set of 
wrappers - it shouldn't kill GC
-put.add(kvBuilder.buildPut(rowKey, 
ref.getFamilyWritable(), cq, ts, value));
+if(this.isLocalIndex) {
+ColumnReference columnReference = 
this.coveredColumnsMap.get(ref);
+   put.add(kvBuilder.buildPut(rowKey, 
columnReference.getFamilyWritable(), cq, ts, value));
--- End diff --

Tell me more about this. We keep a mapping on the client-side for column 
families? I thought we could generate the cf of the local index based on the cf 
of the data table? Is this to save re-generating the column family name all the 
time? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62698174
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/execute/DelegateHTable.java ---
@@ -297,4 +297,28 @@ public boolean checkAndDelete(byte[] row, byte[] 
family, byte[] qualifier,
return delegate.checkAndDelete(row, family, qualifier, 
compareOp, value, delete);
}
 
+@Override
--- End diff --

These should call the method on the delegate.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62698009
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
 ---
@@ -202,7 +207,10 @@ protected RegionScanner doPostScannerOpen(final 
ObserverContext

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62697744
  
--- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
 ---
@@ -165,6 +165,9 @@ public RegionScanner 
preScannerOpen(ObserverContext

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on a diff in the pull request:

https://github.com/apache/phoenix/pull/168#discussion_r62696240
  
--- Diff: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinIT.java ---
@@ -187,13 +186,13 @@ public void initTable() throws Exception {
 "CREATE LOCAL INDEX \"idx_supplier\" ON " + 
JOIN_SUPPLIER_TABLE_FULL_NAME + " (name)"
 }, {
 "SORT-MERGE-JOIN (LEFT) TABLES\n" +
-"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + 
MetaDataUtil.LOCAL_INDEX_TABLE_PREFIX + JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " 
[-32768]\n" +
+"CLIENT PARALLEL 1-WAY RANGE SCAN OVER " 
+JOIN_SUPPLIER_TABLE_DISPLAY_NAME + " [1]\n" +
--- End diff --

We should update the explain plan code to output something else when a 
local index is used, otherwise it's too difficult to tell. Is the local index 
being used?

Something like ... OVER LOCAL INDEX  ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[JamesRTaylor]

Github user JamesRTaylor commented on the pull request:

https://github.com/apache/phoenix/pull/168#issuecomment-218194916
  
How about a version that doesn't depend on HBASE-15600 so that we can 
continue to support older versions of HBase (and can do a 4.8 release)? What 
we've typically done, is once the change makes it to an HBase release, we 
update the pom on that version but provide a fallback implementation 
(conditional on the HBase version at runtime). Then we document that writes 
aren't transactional unless you're using HBase version XXX or above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-1734 Local index improvements(Rajesh...

[chrajeshbabu]

GitHub user chrajeshbabu opened a pull request:

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

PHOENIX-1734 Local index improvements(Rajeshbabu)

This is the patch for new implementation of local index where we store 
local index data in the separate column families in the same table than 
different table. 

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

$ git pull https://github.com/chrajeshbabu/phoenix master

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

https://github.com/apache/phoenix/pull/168.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 #168


commit 2dbb361551b48b96d8335c12b397a2258c266fd3
Author: Rajeshbabu Chintaguntla 
Date:   2016-05-10T14:00:41Z

PHOENIX-1734 Local index improvements(Rajeshbabu)




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---