[GitHub] phoenix pull request #303: PHOENIX-3534 Support multi region SYSTEM.CATALOG ...
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/303#discussion_r191948329 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/DropColumnIT.java --- @@ -522,7 +523,8 @@ public void helpTestDroppingIndexedColDropsViewIndex(boolean isMultiTenant) thro byte[] cq = column.getColumnQualifierBytes(); // there should be a single row belonging to VIEWINDEX2 assertNotNull(viewIndex2 + " row is missing", result.getValue(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, cq)); -assertNull(results.next()); +// TODO enable this after we drop view indexes than need a dropped column +//assertNull(results.next()); --- End diff -- what about this TODO? ---
[GitHub] phoenix pull request #303: PHOENIX-3534 Support multi region SYSTEM.CATALOG ...
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/303#discussion_r191948951 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/WhereConstantParser.java --- @@ -0,0 +1,113 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.phoenix.coprocessor; + +import static org.apache.phoenix.util.PhoenixRuntime.CONNECTIONLESS; +import static org.apache.phoenix.util.PhoenixRuntime.JDBC_PROTOCOL; +import static org.apache.phoenix.util.PhoenixRuntime.JDBC_PROTOCOL_SEPARATOR; + +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.BitSet; +import java.util.List; + +import org.apache.phoenix.compile.ColumnResolver; +import org.apache.phoenix.compile.CreateTableCompiler; +import org.apache.phoenix.compile.ExpressionCompiler; +import org.apache.phoenix.compile.FromCompiler; +import org.apache.phoenix.compile.StatementContext; +import org.apache.phoenix.compile.WhereCompiler; +import org.apache.phoenix.expression.Expression; +import org.apache.phoenix.jdbc.PhoenixConnection; +import org.apache.phoenix.jdbc.PhoenixStatement; +import org.apache.phoenix.parse.ParseNode; +import org.apache.phoenix.parse.SQLParser; +import org.apache.phoenix.parse.SelectStatement; +import org.apache.phoenix.schema.ColumnNotFoundException; +import org.apache.phoenix.schema.PColumn; +import org.apache.phoenix.schema.PColumnImpl; +import org.apache.phoenix.schema.PTable; +import org.apache.phoenix.schema.PTableImpl; +import org.apache.phoenix.schema.TableRef; +import org.apache.phoenix.util.MetaDataUtil; + +import com.google.common.collect.Lists; + + +public class WhereConstantParser { + +static PTable addViewInfoToPColumnsIfNeeded(PTable view) throws SQLException { + boolean[] viewColumnConstantsMatched = new boolean[view.getColumns().size()]; +byte[][] viewColumnConstantsToBe = new byte[view.getColumns().size()][]; +if (view.getViewStatement() == null) { + return view; +} +SelectStatement select = new SQLParser(view.getViewStatement()).parseQuery(); +ParseNode whereNode = select.getWhere(); +ColumnResolver resolver = FromCompiler.getResolver(new TableRef(view)); +StatementContext context = new StatementContext(new PhoenixStatement(getConnectionlessConnection()), resolver); +Expression expression = null; +try { + expression = WhereCompiler.compile(context, whereNode); +} +catch (ColumnNotFoundException e) { + // if we could not find a column used in the view statement (which means its was dropped) + // this view is not valid any more + return null; +} +CreateTableCompiler.ViewWhereExpressionVisitor visitor = +new CreateTableCompiler.ViewWhereExpressionVisitor(view, viewColumnConstantsToBe); +expression.accept(visitor); + +BitSet isViewColumnReferencedToBe = new BitSet(view.getColumns().size()); +// Used to track column references in a view +ExpressionCompiler expressionCompiler = new CreateTableCompiler.ColumnTrackingExpressionCompiler(context, isViewColumnReferencedToBe); +whereNode.accept(expressionCompiler); + +List result = Lists.newArrayList(); +for (PColumn column : PTableImpl.getColumnsToClone(view)) { + boolean isViewReferenced = isViewColumnReferencedToBe.get(column.getPosition()); + if ( (visitor.isUpdatable() || view.getPKColumns().get(MetaDataUtil.getAutoPartitionColIndex(view)).equals(column)) + && viewColumnConstantsToBe[column.getPosition()] != null) { + result.add(new PColumnImpl(column, viewColumnConstantsToBe[column.getP
[GitHub] phoenix pull request #303: PHOENIX-3534 Support multi region SYSTEM.CATALOG ...
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/303#discussion_r191948812 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java --- @@ -787,7 +787,7 @@ public MutationPlan compile(UpsertStatement upsert) throws SQLException { LinkedHashSet updateColumns = Sets.newLinkedHashSetWithExpectedSize(nColumns + 1); updateColumns.add(new PColumnImpl( table.getPKColumns().get(position).getName(), // Use first PK column name as we know it won't conflict with others -null, PVarbinary.INSTANCE, null, null, false, position, SortOrder.getDefault(), 0, null, false, null, false, false, null)); +null, PVarbinary.INSTANCE, null, null, false, position, SortOrder.getDefault(), 0, null, false, null, false, false, null, table.getPKColumns().get(0).getTimestamp())); --- End diff -- are we guaranteed to get a non null column back? ---
[GitHub] phoenix pull request #303: PHOENIX-3534 Support multi region SYSTEM.CATALOG ...
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/303#discussion_r191948510 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/ColumnNameTrackingExpressionCompiler.java --- @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.phoenix.compile; + +import java.sql.SQLException; +import java.util.List; + +import org.apache.phoenix.parse.ColumnParseNode; +import org.apache.phoenix.parse.StatelessTraverseAllParseNodeVisitor; + +import com.google.common.collect.Lists; + +public class ColumnNameTrackingExpressionCompiler extends StatelessTraverseAllParseNodeVisitor { + + private List dataColumnNames = Lists.newArrayListWithExpectedSize(10); + +public void reset() { +this.getDataColumnNames().clear(); +} + + @Override +public Void visit(ColumnParseNode node) throws SQLException { + getDataColumnNames().add(node.getName()); +return null; +} + + public List getDataColumnNames() { + return dataColumnNames; + } + +} --- End diff -- Nit, but indentation seems off for this class. ---
[GitHub] phoenix pull request #248: Phoenix 3534
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/248#discussion_r117566388 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java --- @@ -107,12 +107,14 @@ public void testAddNewColumnsToBaseTableWithViews() throws Exception { assertTableDefinition(conn, tableName, PTableType.TABLE, null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2"); viewConn.createStatement().execute("CREATE VIEW " + viewOfTable + " ( VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR ) AS SELECT * FROM " + tableName); -assertTableDefinition(conn, viewOfTable, PTableType.VIEW, tableName, 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2"); +assertTableDefinition(conn, viewOfTable, PTableType.VIEW, tableName, 0, 5, 3, "VIEW_COL1", "VIEW_COL2"); --- End diff -- Sure we can do that, I think we will need to modify a ton of tests to make this happen. Its not only done here. Another thing is that there are a few tests where the assumptions of how the system works has changed. A few examples are tests like this: ViewIT.testDisallowDropOfColumnOnParentTable() ViewIT.testDisallowDropOfReferencedColumn() QueryCompiler.testDuplicatePKColumn() (this one because we have to support both the diff based approach and the current approach for our migration step, we remove duplicate columns on read, thus we don't prevent this from happening). There are quite a few more, we can maybe discuss if these tests are no longer valid or how we can change them to make them meaningful. --- 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 #236: Loadbalancer
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/236#discussion_r117542085 --- Diff: phoenix-queryserver/pom.xml --- @@ -147,6 +147,14 @@ commons-logging commons-logging + + org.apache.phoenix + phoenix-queryserver-loadbalancer --- End diff -- how do you propose sharing these configurations, which is needed by the load balancer and PQS? --- 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 issue #248: Phoenix 3534
Github user churrodog commented on the issue: https://github.com/apache/phoenix/pull/248 @twdsilva would you mind taking a look 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 #248: Phoenix 3534
GitHub user churrodog opened a pull request: https://github.com/apache/phoenix/pull/248 Phoenix 3534 I'll squash these down after a review You can merge this pull request into a Git repository by running: $ git pull https://github.com/churrodog/phoenix PHOENIX-3534 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/248.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 #248 commit a7712e3977830ffe715a2caaa577eb5cdb071e90 Author: rgidwani Date: 2017-03-17T19:21:56Z Starting work on splittable System.Catalog commit 220849fff34211a4b6356200365afe81d639cfc1 Author: rgidwani Date: 2017-03-20T20:44:54Z Removed all references to multi-region System.Catalog for TableViewFinderResult commit 356cd43e20be0d01c7ef5f97a9b9d26e213a140d Author: rgidwani Date: 2017-03-31T23:11:59Z Create table work, still trying to get rid of all columns commit e01adb5f1c45d43a684371dcbf39eadbc381f9d2 Author: rgidwani Date: 2017-04-04T20:33:07Z Create table and read views work now commit 19c7ce54dcd1616ca8b8a7078fa8fc738e70f4f4 Author: rgidwani Date: 2017-04-04T20:35:30Z Fixed the test - moving on to add drop columns commit ab20f8da00145cc527c6f78dc7d493df7ac2f1b0 Author: rgidwani Date: 2017-04-04T22:59:10Z getting tests and add column to work commit ec3574453e8fae662a271f4340f82eaf90f52ce2 Author: rgidwani Date: 2017-04-05T23:02:29Z Figuring out the delete logic and refactoring the old tests commit 7d4133034b7167a7919eb9dd4ab19533ae9300ea Author: rgidwani Date: 2017-04-11T22:25:32Z Added proto timestamp and exluded values to pcolumn also took care of additive case where we take lower timestamp commit adfc5ce7f9e7f55f801b5b8af5f414fd7ff96c23 Author: rgidwani Date: 2017-04-28T23:02:27Z Drop Column Work in Progress, need to figure out how to resolve the combine logic for excluded columns commit 24414bd942055769b68943d93e19013777ff7299 Author: rgidwani Date: 2017-05-01T22:32:35Z Alter view drop column works! commit 13b6e520e3f7a81bb257c7fe68bae81ededa6c99 Author: rgidwani Date: 2017-05-12T22:55:49Z Drop Cascade and create check completed, need to test commit 0313ceee0b0a7515323dc0fc402a6eac76786155 Author: rgidwani Date: 2017-05-15T20:40:14Z Drop cascade seems to work commit 86a2633f57ec541330e56fd9c3a5da20a9cce165 Author: rgidwani Date: 2017-05-15T21:13:30Z First pass at multi-region System.CATALOG --- 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-2822 - Tests that extend BaseHBaseMa...
Github user churrodog commented on the pull request: https://github.com/apache/phoenix/pull/158#issuecomment-207610636 @samarthjain rebased and put patch up here: https://issues.apache.org/jira/browse/PHOENIX-2822 . Thanks for the review. --- 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-2822 - Tests that extend BaseHBaseMa...
Github user churrodog commented on the pull request: https://github.com/apache/phoenix/pull/158#issuecomment-207521158 made the changes you requested. --- 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-2822 - Tests that extend BaseHBaseMa...
Github user churrodog commented on the pull request: https://github.com/apache/phoenix/pull/158#issuecomment-206450571 I also un-ignored the memory manager test in the last commit and fixed it up to remove the long sleeps. --- 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-2822 - Tests that extend BaseHBaseMa...
Github user churrodog commented on a diff in the pull request: https://github.com/apache/phoenix/pull/158#discussion_r58735016 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/AbsFunctionEnd2EndIT.java --- @@ -43,9 +44,10 @@ public void initTable() throws Exception { Connection conn = null; PreparedStatement stmt = null; try { +System.out.println("TABLE_NAME = " + TABLE_NAME); --- End diff -- no problem --- 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-2822 - Tests that extend BaseHBaseMa...
GitHub user churrodog opened a pull request: https://github.com/apache/phoenix/pull/158 PHOENIX-2822 - Tests that extend BaseHBaseManagedTimeIT are very slow You can merge this pull request into a Git repository by running: $ git pull https://github.com/churrodog/phoenix PHOENIX-2822 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/158.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 #158 commit 8325d00cd01b2a7d71b52b7c6d0ff5f6147ba47a Author: rahul gidwani Date: 2016-04-05T19:38:21Z PHOENIX-2822 - Tests that extend BaseHBaseManagedTimeIT are very slow Conflicts: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java --- 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. ---