You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2021/07/22 14:15:05 UTC

[GitHub] [phoenix] stoty commented on a change in pull request #1271: PHOENIX-6518 Implement SHOW CREATE STATEMENT SQL command

stoty commented on a change in pull request #1271:
URL: https://github.com/apache/phoenix/pull/1271#discussion_r674825398



##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ShowCreateStatementIT.java
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.end2end;
+
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.util.Properties;
+
+import static org.junit.Assert.assertTrue;
+
+public class ShowCreateStatementIT extends ParallelStatsDisabledIT {
+    @Test
+    public void testDescribebasic() throws Exception {

Review comment:
       not describe

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ShowCreateStatementIT.java
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.end2end;
+
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.util.Properties;
+
+import static org.junit.Assert.assertTrue;
+
+public class ShowCreateStatementIT extends ParallelStatsDisabledIT {
+    @Test
+    public void testDescribebasic() throws Exception {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        String ddl = "CREATE TABLE " + tableName + "(K VARCHAR NOT NULL PRIMARY KEY, INT INTEGER)";
+        conn.createStatement().execute(ddl);
+
+        ResultSet rs = conn.createStatement().executeQuery("SHOW CREATE STATEMENT " + tableName );
+        assertTrue(rs.next());
+        System.out.println(ddl);
+        System.out.println(rs.getString(1));
+        assertTrue(rs.getString(1).contains(ddl));
+    }
+
+    @Ignore
+    @Test
+    public void testDescribeLowerCase() throws Exception {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = "lowercasetbl1";
+        String ddl = "CREATE TABLE \"" + tableName + "\"(K VARCHAR NOT NULL PRIMARY KEY, INT INTEGER)";
+        conn.createStatement().execute(ddl);
+
+        ResultSet rs = conn.createStatement().executeQuery("SHOW CREATE STATEMENT \"" + tableName + "\"");
+        assertTrue(rs.next());
+        System.out.println(ddl);
+        System.out.println(rs.getString(1));
+        assertTrue(rs.getString(1).contains(ddl));
+    }
+
+    @Test
+    public void testDescribeUpperCase() throws Exception {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String tableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String ddl = "CREATE TABLE " + tableFullName + "(K VARCHAR NOT NULL PRIMARY KEY, INT INTEGER)";
+        conn.createStatement().execute(ddl);
+
+        ResultSet rs = conn.createStatement().executeQuery("SHOW CREATE STATEMENT " + tableFullName + "");
+        assertTrue(rs.next());
+        System.out.println(ddl);

Review comment:
       debug printlns
   

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/parse/ShowCreateTablesStatement.java
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.parse;
+
+import org.apache.phoenix.compile.ColumnResolver;
+import org.apache.phoenix.thirdparty.com.google.common.base.Preconditions;
+import org.apache.phoenix.util.SchemaUtil;
+
+import java.util.Objects;
+
+/**
+ * ParseNode implementation for DESCRIBE tablename.
+ */
+public class ShowCreateTablesStatement extends ShowCreateStatement {
+
+    private TableName tableName;
+
+    public ShowCreateTablesStatement(TableName tn) {
+        tableName = tn;
+    }
+
+    public TableName getTableName() {
+        return tableName;
+    }
+
+    public void toSQL(ColumnResolver resolver, StringBuilder buf) {
+        Preconditions.checkNotNull(buf);
+        buf.append("SHOW CREATE STATEMENT ");
+
+        if(tableName.getSchemaName() != null) {
+            if (tableName.isSchemaNameCaseSensitive()) {
+                buf.append("\"");
+            }
+            buf.append(tableName.getSchemaName());
+            if (tableName.isSchemaNameCaseSensitive()) {
+                buf.append("\"");
+            }
+            buf.append(".");
+        }
+
+        if (tableName.isTableNameCaseSensitive()) {
+            buf.append("\"");
+        }
+        buf.append(tableName.getTableName());
+        if (tableName.isTableNameCaseSensitive()) {
+            buf.append("\"");
+        }

Review comment:
       Pretty sure we have a helper function already for this.

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaToolSynthesisIT.java
##########
@@ -23,8 +23,8 @@
 import java.io.File;
 import java.net.URL;
 
-import static org.apache.phoenix.schema.SchemaSynthesisProcessor.ENTITY_NAME_IN_BASE_AND_ALTER_DDL_DON_T_MATCH;
-import static org.apache.phoenix.schema.SchemaSynthesisProcessor.UNSUPPORTED_DDL_EXCEPTION;
+import static org.apache.phoenix.schematool.SchemaSynthesisProcessor.ENTITY_NAME_IN_BASE_AND_ALTER_DDL_DON_T_MATCH;

Review comment:
       Not sure about the "schematool" package name.
   Perhaps "schemaextract" or "sema.generate" or similar would be more descriptive.
   

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ShowCreateStatementIT.java
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.end2end;
+
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.util.Properties;
+
+import static org.junit.Assert.assertTrue;
+
+public class ShowCreateStatementIT extends ParallelStatsDisabledIT {
+    @Test
+    public void testDescribebasic() throws Exception {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        String ddl = "CREATE TABLE " + tableName + "(K VARCHAR NOT NULL PRIMARY KEY, INT INTEGER)";
+        conn.createStatement().execute(ddl);
+
+        ResultSet rs = conn.createStatement().executeQuery("SHOW CREATE STATEMENT " + tableName );
+        assertTrue(rs.next());
+        System.out.println(ddl);
+        System.out.println(rs.getString(1));
+        assertTrue(rs.getString(1).contains(ddl));
+    }
+
+    @Ignore
+    @Test
+    public void testDescribeLowerCase() throws Exception {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = "lowercasetbl1";
+        String ddl = "CREATE TABLE \"" + tableName + "\"(K VARCHAR NOT NULL PRIMARY KEY, INT INTEGER)";
+        conn.createStatement().execute(ddl);
+
+        ResultSet rs = conn.createStatement().executeQuery("SHOW CREATE STATEMENT \"" + tableName + "\"");
+        assertTrue(rs.next());
+        System.out.println(ddl);

Review comment:
       do not commit printlns

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
##########
@@ -1302,6 +1304,22 @@ public QueryPlan compilePlan(final PhoenixStatement stmt, Sequence.ValueOp seqAc
         }
     }
 
+    private static class ExecutableShowCreateTablesStatement extends ShowCreateTablesStatement

Review comment:
       class names don't match the command name

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/parse/ShowCreateTablesStatement.java
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.parse;
+
+import org.apache.phoenix.compile.ColumnResolver;
+import org.apache.phoenix.thirdparty.com.google.common.base.Preconditions;
+import org.apache.phoenix.util.SchemaUtil;
+
+import java.util.Objects;
+
+/**
+ * ParseNode implementation for DESCRIBE tablename.

Review comment:
       does not match command 

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/QueryUtil.java
##########
@@ -769,6 +772,42 @@ public static PreparedStatement getTablesStmt(PhoenixConnection connection, Stri
         return stmt;
     }
 
+    /**
+     * Util that generates a PreparedStatement against syscat to get the table listing in a given schema.
+     */
+    public static PreparedStatement getShowCreateStatementStmt(PhoenixConnection connection, String catalog, TableName tn) throws SQLException {
+
+        String output;
+        SchemaProcessor processor = new SchemaExtractionProcessor(null,
+                connection.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration(),
+                tn.getSchemaName() == null ? null : "\"" + tn.getSchemaName()+ "\"",
+                "\"" + tn.getTableName() + "\"");
+        try {
+            output = processor.process();
+        } catch (Exception e) {
+            LOGGER.error(e.getStackTrace().toString());
+            throw new SQLException(e.getMessage());
+        }
+
+        String stmtType;

Review comment:
       Not sure we need to adjust the column name.
   "CREATE STATEMENT" is probably enough.

##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ShowCreateStatementIT.java
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.end2end;
+
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.util.Properties;
+
+import static org.junit.Assert.assertTrue;
+
+public class ShowCreateStatementIT extends ParallelStatsDisabledIT {
+    @Test
+    public void testDescribebasic() throws Exception {
+        Properties props = new Properties();
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        String tableName = generateUniqueName();
+        String ddl = "CREATE TABLE " + tableName + "(K VARCHAR NOT NULL PRIMARY KEY, INT INTEGER)";
+        conn.createStatement().execute(ddl);
+
+        ResultSet rs = conn.createStatement().executeQuery("SHOW CREATE STATEMENT " + tableName );
+        assertTrue(rs.next());
+        System.out.println(ddl);

Review comment:
       remove debug printlns

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/parse/ShowCreateStatement.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.parse;
+
+import org.apache.phoenix.jdbc.PhoenixStatement;
+
+/**
+ * Parent class for all SHOW statements. SHOW SCHEMAS, SHOW TABLES etc.

Review comment:
       Comment doesn't seem to be true




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

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org