From issues-return-6918-archive-asf-public=cust-asf.ponee.io@phoenix.apache.org Mon May 20 16:34:23 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id E5DA718076D for ; Mon, 20 May 2019 18:34:22 +0200 (CEST) Received: (qmail 22493 invoked by uid 500); 20 May 2019 16:34:21 -0000 Mailing-List: contact issues-help@phoenix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@phoenix.apache.org Delivered-To: mailing list issues@phoenix.apache.org Received: (qmail 22430 invoked by uid 99); 20 May 2019 16:34:21 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 20 May 2019 16:34:21 +0000 From: GitBox To: issues@phoenix.apache.org Subject: =?utf-8?q?=5BGitHub=5D_=5Bphoenix=5D_priyankporwal_commented_on_a_change_?= =?utf-8?q?in_pull_request_=23508=3A_PHOENIX-5283=3A_Add_CASCADE_INDEX_ALL?= =?utf-8?q?_in_the_SQL_Grammar_of_ALTER_TABLE=E2=80=A6?= Message-ID: <155837005674.10352.17869715538563985047.gitbox@gitbox.apache.org> Date: Mon, 20 May 2019 16:34:16 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit priyankporwal commented on a change in pull request #508: PHOENIX-5283: Add CASCADE INDEX ALL in the SQL Grammar of ALTER TABLE… URL: https://github.com/apache/phoenix/pull/508#discussion_r285676669 ########## File path: phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java ########## @@ -3524,14 +3525,26 @@ private void mutateStringProperty(String tenantId, String schemaName, String tab public MutationState addColumn(AddColumnStatement statement) throws SQLException { PTable table = FromCompiler.getResolver(statement, connection).getTables().get(0).getTable(); - return addColumn(table, statement.getColumnDefs(), statement.getProps(), statement.ifNotExists(), false, statement.getTable(), statement.getTableType()); + return addColumn(table, statement.getColumnDefs(), statement.getProps(), statement.ifNotExists(), false, statement.getTable(), statement.getTableType(), statement.isCascade(), statement.getIndexes()); } public MutationState addColumn(PTable table, List origColumnDefs, ListMultimap> stmtProperties, boolean ifNotExists, - boolean removeTableProps, NamedTableNode namedTableNode, PTableType tableType) + boolean removeTableProps, NamedTableNode namedTableNode, PTableType tableType, boolean cascade, List indexes) throws SQLException { connection.rollback(); + if (cascade && (indexes == null || indexes.size()>0)) { Review comment: The condition (indexes==null || indexes.size()>0) seems very un-intuitive. It requires folks to understand that indexes==null is the case with ALL and the latter is when there is explicit list of indexes; which is not very clear just reading this code. I'd just remove this condition entirely for now. It is not relevant for the throwing NOT_SUPPORTED_CASCADE_FEATURE exception for views. And indexes should be properly handled when the feature is implemented. ---------------------------------------------------------------- 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: users@infra.apache.org With regards, Apache Git Services