You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/07/11 12:00:52 UTC

[GitHub] [flink] godfreyhe opened a new pull request, #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

godfreyhe opened a new pull request, #20242:
URL: https://github.com/apache/flink/pull/20242

   
   ## What is the purpose of the change
   
   *Introduce `ANALYZE TABLE` syntax in sql parser, see for detail from https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=217386481*
   
   
   ## Brief change log
   
     - *Introduce `ANALYZE TABLE` syntax in sql parser*
   
   
   ## Verifying this change
   
   
   
   This change added tests and can be verified as follows:
   
     - *Extended FlinkSqlParserImplTest to verify different ANALYZE TABLE statements*
   
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (yes / **no**)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (**yes** / no)
     - The serializers: (yes / **no** / don't know)
     - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / **no** / don't know)
     - The S3 file system connector: (yes / **no** / don't know)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (**yes** / no)
     - If yes, how is the feature documented? (not applicable / docs / JavaDocs / **not documented)**
   


-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on a diff in pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r921982228


##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,81 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPair PartitionKeyValuePair():
+{
+    SqlIdentifier key;
+    SqlNode value = null;
+    SqlParserPos pos;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+           return new SqlPair(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPair keyValuePair;
+}
+{
+    <LPAREN>
+    keyValuePair = PartitionKeyValuePair()
+    {
+       list.add(keyValuePair);
+    }
+    (
+        <COMMA> keyValuePair = PartitionKeyValuePair()
+        {
+            list.add(keyValuePair);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       Span s;
+       SqlIdentifier tableName;
+       SqlNodeList partitionSpec = null;
+       SqlNodeList columns = null;
+       boolean allColumns = false;
+}
+{
+    <ANALYZE> <TABLE> { s = span(); }
+    tableName = CompoundIdentifier()
+    [
+        <PARTITION> {
+            partitionSpec = new SqlNodeList(getPos());
+            ExtendedPartitionSpecCommaList(partitionSpec);
+        }
+    ]
+
+    <COMPUTE> <STATISTICS> [
+        (

Review Comment:
   The parser can't chose the correct branch if `LOOKAHEAD` is missing



-- 
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@flink.apache.org

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


[GitHub] [flink] lsyldliu commented on a diff in pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r921761281


##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,81 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPair PartitionKeyValuePair():
+{
+    SqlIdentifier key;
+    SqlNode value = null;
+    SqlParserPos pos;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+           return new SqlPair(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPair keyValuePair;
+}
+{
+    <LPAREN>
+    keyValuePair = PartitionKeyValuePair()
+    {
+       list.add(keyValuePair);
+    }
+    (
+        <COMMA> keyValuePair = PartitionKeyValuePair()
+        {
+            list.add(keyValuePair);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       Span s;

Review Comment:
   final



##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,81 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPair PartitionKeyValuePair():
+{
+    SqlIdentifier key;
+    SqlNode value = null;
+    SqlParserPos pos;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+           return new SqlPair(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPair keyValuePair;
+}
+{
+    <LPAREN>
+    keyValuePair = PartitionKeyValuePair()
+    {
+       list.add(keyValuePair);
+    }
+    (
+        <COMMA> keyValuePair = PartitionKeyValuePair()
+        {
+            list.add(keyValuePair);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       Span s;
+       SqlIdentifier tableName;
+       SqlNodeList partitionSpec = null;
+       SqlNodeList columns = null;
+       boolean allColumns = false;
+}
+{
+    <ANALYZE> <TABLE> { s = span(); }
+    tableName = CompoundIdentifier()
+    [
+        <PARTITION> {
+            partitionSpec = new SqlNodeList(getPos());
+            ExtendedPartitionSpecCommaList(partitionSpec);
+        }
+    ]
+
+    <COMPUTE> <STATISTICS> [
+        (

Review Comment:
   > 
           <FOR>    
           (
              <COLUMNS> { columns = ParenthesizedSimpleIdentifierList(); }
           |
              <ALL> <COLUMNS> { allColumns = true; }
           )



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);

Review Comment:
   The element type in partitions  is `SqlPair`, call this method will occur error.



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlPair.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.flink.sql.parser;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Properties of PartitionSpec, a key-value pair with key as component identifier and value as
+ * string literal. Different from {@link SqlProperty}, {@link SqlPair} allows the value is null.
+ */
+public class SqlPair extends SqlCall {

Review Comment:
   If you can't get a generic class name, I think use a name suitable for its use case may also be better. Maybe `SqlPartitionSpecProperty` more meaningful?



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);

Review Comment:
   This is not a DDL, maybe `SqlKind.OTHER` is more suitable? Spark archive this to auxiliary statements
   



##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,81 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPair PartitionKeyValuePair():
+{
+    SqlIdentifier key;
+    SqlNode value = null;
+    SqlParserPos pos;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+           return new SqlPair(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPair keyValuePair;
+}
+{
+    <LPAREN>
+    keyValuePair = PartitionKeyValuePair()
+    {
+       list.add(keyValuePair);
+    }
+    (
+        <COMMA> keyValuePair = PartitionKeyValuePair()
+        {
+            list.add(keyValuePair);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       Span s;
+       SqlIdentifier tableName;
+       SqlNodeList partitionSpec = null;
+       SqlNodeList columns = null;

Review Comment:
   ```suggestion
          SqlNodeList columns = SqlNodeList.EMPTY;
   ```



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {
+        if (columns == null) {
+            return new String[0];
+        }
+        return columns.getList().stream()
+                .map(col -> ((SqlIdentifier) col).getSimple())
+                .toArray(String[]::new);
+    }
+
+    public boolean isAllColumns() {
+        return allColumns;
+    }
+
+    @Nonnull
+    @Override
+    public SqlOperator getOperator() {
+        return OPERATOR;
+    }
+
+    @Nonnull
+    @Override
+    public List<SqlNode> getOperandList() {
+        return ImmutableNullableList.of(tableName, partitions, columns);
+    }
+
+    public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
+        writer.keyword("ANALYZE");
+        writer.keyword("TABLE");
+        final int opLeft = getOperator().getLeftPrec();
+        final int opRight = getOperator().getRightPrec();
+        tableName.unparse(writer, opLeft, opRight);
+
+        if (partitions != null && partitions.size() > 0) {
+            writer.keyword("PARTITION");
+            partitions.unparse(writer, opLeft, opRight);
+        }
+
+        writer.keyword("COMPUTE");
+        writer.keyword("STATISTICS");
+
+        if (columns != null && columns.size() > 0) {
+            writer.keyword("FOR");
+            writer.keyword("COLUMNS");
+            columns.unparse(writer, opLeft, opRight);
+        }
+        if (allColumns) {
+            writer.keyword("FOR");

Review Comment:
   It would be better `writer.keyword("FOR ALL COLUMNS");`



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {
+        if (columns == null) {
+            return new String[0];
+        }
+        return columns.getList().stream()
+                .map(col -> ((SqlIdentifier) col).getSimple())
+                .toArray(String[]::new);
+    }
+
+    public boolean isAllColumns() {
+        return allColumns;
+    }
+
+    @Nonnull
+    @Override
+    public SqlOperator getOperator() {
+        return OPERATOR;
+    }
+
+    @Nonnull
+    @Override
+    public List<SqlNode> getOperandList() {
+        return ImmutableNullableList.of(tableName, partitions, columns);
+    }
+
+    public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
+        writer.keyword("ANALYZE");
+        writer.keyword("TABLE");
+        final int opLeft = getOperator().getLeftPrec();
+        final int opRight = getOperator().getRightPrec();
+        tableName.unparse(writer, opLeft, opRight);
+
+        if (partitions != null && partitions.size() > 0) {
+            writer.keyword("PARTITION");
+            partitions.unparse(writer, opLeft, opRight);
+        }
+
+        writer.keyword("COMPUTE");

Review Comment:
   It would better `writer.keyword("COMPUTE STATISTICS");`



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {

Review Comment:
   Why not return List?



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {
+        if (columns == null) {

Review Comment:
   The `columns `  default value is `SqlNodeList.EMPTY`, otherwise you should `@Nullable` annotation.



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {
+        if (columns == null) {
+            return new String[0];
+        }
+        return columns.getList().stream()
+                .map(col -> ((SqlIdentifier) col).getSimple())
+                .toArray(String[]::new);
+    }
+
+    public boolean isAllColumns() {
+        return allColumns;
+    }
+
+    @Nonnull
+    @Override
+    public SqlOperator getOperator() {
+        return OPERATOR;
+    }
+
+    @Nonnull
+    @Override
+    public List<SqlNode> getOperandList() {
+        return ImmutableNullableList.of(tableName, partitions, columns);
+    }
+
+    public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
+        writer.keyword("ANALYZE");
+        writer.keyword("TABLE");
+        final int opLeft = getOperator().getLeftPrec();
+        final int opRight = getOperator().getRightPrec();
+        tableName.unparse(writer, opLeft, opRight);
+
+        if (partitions != null && partitions.size() > 0) {
+            writer.keyword("PARTITION");
+            partitions.unparse(writer, opLeft, opRight);
+        }
+
+        writer.keyword("COMPUTE");
+        writer.keyword("STATISTICS");
+
+        if (columns != null && columns.size() > 0) {
+            writer.keyword("FOR");

Review Comment:
   It would be better `writer.keyword("FOR COLUMNS");`



##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,81 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPair PartitionKeyValuePair():
+{
+    SqlIdentifier key;
+    SqlNode value = null;
+    SqlParserPos pos;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+           return new SqlPair(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPair keyValuePair;
+}
+{
+    <LPAREN>
+    keyValuePair = PartitionKeyValuePair()
+    {
+       list.add(keyValuePair);
+    }
+    (
+        <COMMA> keyValuePair = PartitionKeyValuePair()
+        {
+            list.add(keyValuePair);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       Span s;
+       SqlIdentifier tableName;
+       SqlNodeList partitionSpec = null;

Review Comment:
   ```suggestion
          SqlNodeList partitionSpecs = SqlNodeList.EMPTY;
   ```



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {
+        if (columns == null) {
+            return new String[0];
+        }
+        return columns.getList().stream()
+                .map(col -> ((SqlIdentifier) col).getSimple())
+                .toArray(String[]::new);
+    }
+
+    public boolean isAllColumns() {
+        return allColumns;
+    }
+
+    @Nonnull
+    @Override
+    public SqlOperator getOperator() {
+        return OPERATOR;
+    }
+
+    @Nonnull
+    @Override
+    public List<SqlNode> getOperandList() {
+        return ImmutableNullableList.of(tableName, partitions, columns);
+    }
+
+    public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
+        writer.keyword("ANALYZE");

Review Comment:
   It would be better `writer.keyword("ANALYZE TABLE");`



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlPair.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.flink.sql.parser;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Properties of PartitionSpec, a key-value pair with key as component identifier and value as
+ * string literal. Different from {@link SqlProperty}, {@link SqlPair} allows the value is null.
+ */
+public class SqlPair extends SqlCall {
+
+    /** Use this operator only if you don't have a better one. */
+    protected static final SqlOperator OPERATOR = new SqlSpecialOperator("Pair", SqlKind.OTHER);
+
+    private final SqlIdentifier key;
+    private final SqlNode value;

Review Comment:
   @Nullable



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;

Review Comment:
   The `partitions ` default value is `SqlNodeList.EMPTY`, otherwise you should @Nullable annotation.



-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe closed pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe closed pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser
URL: https://github.com/apache/flink/pull/20242


-- 
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@flink.apache.org

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


[GitHub] [flink] lsyldliu commented on a diff in pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r922068090


##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,81 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPair PartitionKeyValuePair():
+{
+    SqlIdentifier key;
+    SqlNode value = null;
+    SqlParserPos pos;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+           return new SqlPair(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPair keyValuePair;
+}
+{
+    <LPAREN>
+    keyValuePair = PartitionKeyValuePair()
+    {
+       list.add(keyValuePair);
+    }
+    (
+        <COMMA> keyValuePair = PartitionKeyValuePair()
+        {
+            list.add(keyValuePair);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       Span s;
+       SqlIdentifier tableName;
+       SqlNodeList partitionSpec = null;
+       SqlNodeList columns = null;
+       boolean allColumns = false;
+}
+{
+    <ANALYZE> <TABLE> { s = span(); }
+    tableName = CompoundIdentifier()
+    [
+        <PARTITION> {
+            partitionSpec = new SqlNodeList(getPos());
+            ExtendedPartitionSpecCommaList(partitionSpec);
+        }
+    ]
+
+    <COMPUTE> <STATISTICS> [
+        (

Review Comment:
   I have tested it, it can.



-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on a diff in pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r924466802


##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,94 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPartitionSpecProperty PartitionSpecProperty():
+{
+    final SqlParserPos pos;
+    final SqlIdentifier key;
+    SqlNode value = null;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+        return new SqlPartitionSpecProperty(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPartitionSpecProperty property;
+}
+{
+    <LPAREN>
+    property = PartitionSpecProperty()
+    {
+       list.add(property);
+    }
+    (
+        <COMMA> property = PartitionSpecProperty()
+        {
+            list.add(property);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses a comma-separated list of simple identifiers with position. */
+SqlNodeList SimpleIdentifierCommaListWithPosition() :
+{
+    final Span s;
+    final List<SqlNode> list = new ArrayList<SqlNode>();
+}
+{
+    { s = span(); }
+    SimpleIdentifierCommaList(list) {
+        return new SqlNodeList(list, s.end(this));
+    }
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       final Span s;
+       final SqlIdentifier tableName;
+       SqlNodeList partitionSpec = SqlNodeList.EMPTY;
+       SqlNodeList columns = SqlNodeList.EMPTY;
+       boolean allColumns = false;
+}
+{
+    <ANALYZE> <TABLE> { s = span(); }
+    tableName = CompoundIdentifier()
+    [
+        <PARTITION> {
+            partitionSpec = new SqlNodeList(getPos());
+            ExtendedPartitionSpecCommaList(partitionSpec);

Review Comment:
   Hi @lincoln-lil thanks for the input, I have test your suggestion, but it can't handle the case of key-value pair and key-only, such as: `analyze table emps partition(x='ab', y) compute statistics for all columns`.  which means the table has two partition keys: `x` and `y`, but only `x='ab' ` is required.



-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on a diff in pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r922013489


##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = tableName;
+        this.partitions = partitions;
+        this.columns = columns;
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /** Get partition spec as key-value strings. */
+    public LinkedHashMap<String, String> getPartitionKVs() {
+        return SqlPartitionUtils.getPartitionKVs(partitions);
+    }
+
+    public String[] getColumnNames() {

Review Comment:
   avoid npe



-- 
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@flink.apache.org

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


[GitHub] [flink] flinkbot commented on pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #20242:
URL: https://github.com/apache/flink/pull/20242#issuecomment-1180331411

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "2e9429734066e4ff763abc9af2a1035639bbd905",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "2e9429734066e4ff763abc9af2a1035639bbd905",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 2e9429734066e4ff763abc9af2a1035639bbd905 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on PR #20242:
URL: https://github.com/apache/flink/pull/20242#issuecomment-1181733875

   @flinkbot run azure
   


-- 
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@flink.apache.org

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


[GitHub] [flink] lsyldliu commented on a diff in pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
lsyldliu commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r923121093


##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionSpecProperty;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = requireNonNull(tableName, "tableName is null");
+        this.partitions = requireNonNull(partitions, "partitions is null");
+        this.columns = requireNonNull(columns, "columns is null");
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /**
+     * Get partition spec as key-value strings, if only partition key is given, the corresponding
+     * value is null.
+     */
+    public LinkedHashMap<String, String> getPartitions() {
+        LinkedHashMap<String, String> ret = new LinkedHashMap<>();
+        for (SqlNode node : partitions.getList()) {
+            SqlPartitionSpecProperty property = (SqlPartitionSpecProperty) node;
+            final String value;
+            if (property.getValue() != null) {

Review Comment:
   ```suggestion
               if (property.getValue() == null) {
   ```



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlPartitionSpecProperty.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.flink.sql.parser;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Properties of PartitionSpec, a key-value pair with key as component identifier and value as
+ * string literal. Different from {@link SqlProperty}, {@link SqlPartitionSpecProperty} allows the
+ * value is null.
+ */
+public class SqlPartitionSpecProperty extends SqlCall {
+
+    /** Use this operator only if you don't have a better one. */
+    protected static final SqlOperator OPERATOR = new SqlSpecialOperator("Pair", SqlKind.OTHER);

Review Comment:
   ```suggestion
       protected static final SqlOperator OPERATOR = new SqlSpecialOperator("Partition Spec", SqlKind.OTHER);
   ```



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionSpecProperty;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = requireNonNull(tableName, "tableName is null");
+        this.partitions = requireNonNull(partitions, "partitions is null");
+        this.columns = requireNonNull(columns, "columns is null");
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /**
+     * Get partition spec as key-value strings, if only partition key is given, the corresponding
+     * value is null.
+     */
+    public LinkedHashMap<String, String> getPartitions() {
+        LinkedHashMap<String, String> ret = new LinkedHashMap<>();
+        for (SqlNode node : partitions.getList()) {
+            SqlPartitionSpecProperty property = (SqlPartitionSpecProperty) node;
+            final String value;
+            if (property.getValue() != null) {
+                value = null;
+            } else {
+                Comparable<?> comparable = SqlLiteral.value(property.getValue());
+                value =
+                        comparable instanceof NlsString
+                                ? ((NlsString) comparable).getValue()
+                                : comparable.toString();
+            }
+
+            ret.put(property.getKey().getSimple(), value);
+        }
+        return ret;
+    }
+
+    public String[] getColumnNames() {

Review Comment:
   Return list directly?



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/SqlPartitionSpecProperty.java:
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.flink.sql.parser;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import javax.annotation.Nullable;
+
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Properties of PartitionSpec, a key-value pair with key as component identifier and value as
+ * string literal. Different from {@link SqlProperty}, {@link SqlPartitionSpecProperty} allows the
+ * value is null.
+ */
+public class SqlPartitionSpecProperty extends SqlCall {
+
+    /** Use this operator only if you don't have a better one. */
+    protected static final SqlOperator OPERATOR = new SqlSpecialOperator("Pair", SqlKind.OTHER);
+
+    private final SqlIdentifier key;
+    private final @Nullable SqlNode value;
+
+    public SqlPartitionSpecProperty(SqlIdentifier key, @Nullable SqlNode value, SqlParserPos pos) {
+        super(pos);
+        this.key = requireNonNull(key, "Pair key is missing");

Review Comment:
   ```suggestion
           this.key = requireNonNull(key, "Partition spec key is missing");
   ```



-- 
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@flink.apache.org

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


[GitHub] [flink] lincoln-lil commented on a diff in pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
lincoln-lil commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r925114125


##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,94 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPartitionSpecProperty PartitionSpecProperty():
+{
+    final SqlParserPos pos;
+    final SqlIdentifier key;
+    SqlNode value = null;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+        return new SqlPartitionSpecProperty(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPartitionSpecProperty property;
+}
+{
+    <LPAREN>
+    property = PartitionSpecProperty()
+    {
+       list.add(property);
+    }
+    (
+        <COMMA> property = PartitionSpecProperty()
+        {
+            list.add(property);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses a comma-separated list of simple identifiers with position. */
+SqlNodeList SimpleIdentifierCommaListWithPosition() :
+{
+    final Span s;
+    final List<SqlNode> list = new ArrayList<SqlNode>();
+}
+{
+    { s = span(); }
+    SimpleIdentifierCommaList(list) {
+        return new SqlNodeList(list, s.end(this));
+    }
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       final Span s;
+       final SqlIdentifier tableName;
+       SqlNodeList partitionSpec = SqlNodeList.EMPTY;
+       SqlNodeList columns = SqlNodeList.EMPTY;
+       boolean allColumns = false;
+}
+{
+    <ANALYZE> <TABLE> { s = span(); }
+    tableName = CompoundIdentifier()
+    [
+        <PARTITION> {
+            partitionSpec = new SqlNodeList(getPos());
+            ExtendedPartitionSpecCommaList(partitionSpec);

Review Comment:
   I see.. hint parser doesn't support combined form options. Current version looks to me. Thanks for your explaination @godfreyhe 



-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on a diff in pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r921997040


##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionUtils;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);

Review Comment:
   It change the statistics info in the catalog, Oracle defines it as DDL, see https://docs.oracle.com/cd/B14117_01/server.101/b10759/statements_1001.htm



-- 
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@flink.apache.org

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


[GitHub] [flink] lincoln-lil commented on a diff in pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
lincoln-lil commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r923342921


##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -2203,3 +2203,94 @@ SqlNode TryCastFunctionCall() :
         return operator.createCall(s.end(this), args);
     }
 }
+
+/**
+* Parses a partition key/value,
+* e.g. p or p = '10'.
+*/
+SqlPartitionSpecProperty PartitionSpecProperty():
+{
+    final SqlParserPos pos;
+    final SqlIdentifier key;
+    SqlNode value = null;
+}
+{
+    key = SimpleIdentifier() { pos = getPos(); }
+    [
+        LOOKAHEAD(1)
+        <EQ> value = Literal()
+    ]
+    {
+        return new SqlPartitionSpecProperty(key, value, pos);
+    }
+}
+
+/**
+* Parses a partition specifications statement,
+* e.g. ANALYZE TABLE tbl1 partition(col1='val1', col2='val2') xxx
+* or
+* ANALYZE TABLE tbl1 partition(col1, col2) xxx.
+*/
+void ExtendedPartitionSpecCommaList(SqlNodeList list) :
+{
+    SqlPartitionSpecProperty property;
+}
+{
+    <LPAREN>
+    property = PartitionSpecProperty()
+    {
+       list.add(property);
+    }
+    (
+        <COMMA> property = PartitionSpecProperty()
+        {
+            list.add(property);
+        }
+    )*
+    <RPAREN>
+}
+
+/** Parses a comma-separated list of simple identifiers with position. */
+SqlNodeList SimpleIdentifierCommaListWithPosition() :
+{
+    final Span s;
+    final List<SqlNode> list = new ArrayList<SqlNode>();
+}
+{
+    { s = span(); }
+    SimpleIdentifierCommaList(list) {
+        return new SqlNodeList(list, s.end(this));
+    }
+}
+
+/** Parses an ANALYZE TABLE statement. */
+SqlNode SqlAnalyzeTable():
+{
+       final Span s;
+       final SqlIdentifier tableName;
+       SqlNodeList partitionSpec = SqlNodeList.EMPTY;
+       SqlNodeList columns = SqlNodeList.EMPTY;
+       boolean allColumns = false;
+}
+{
+    <ANALYZE> <TABLE> { s = span(); }
+    tableName = CompoundIdentifier()
+    [
+        <PARTITION> {
+            partitionSpec = new SqlNodeList(getPos());
+            ExtendedPartitionSpecCommaList(partitionSpec);

Review Comment:
   May this be simplified by reusing existing helpers?  
   e.g.,
   ```
           (
               LOOKAHEAD...
               partitionSpec = ParenthesizedKeyValueOptionCommaList()
           |
               LOOKAHEAD...
               partitionSpec = ParenthesizedSimpleIdentifierList()
           )
   ``` 
   The grammar seems a subset of hints 
   ```
   hintComment:
         '/*+' hint [, hint ]* '*/'
   
   hint:
         hintName
     |   hintName '(' optionKey '=' optionVal [, optionKey '=' optionVal ]* ')'
     |   hintName '(' hintOption [, hintOption ]* ')'
   
   optionKey:
         simpleIdentifier
     |   stringLiteral
   
   optionVal:
         stringLiteral
   
   hintOption:
         simpleIdentifier
      |  numericLiteral
      |  stringLiteral
   ```
   



-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on pull request #20242: [FLINK-28489][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on PR #20242:
URL: https://github.com/apache/flink/pull/20242#issuecomment-1185395919

   @lsyldliu thanks for the review, I have updated the pr


-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on PR #20242:
URL: https://github.com/apache/flink/pull/20242#issuecomment-1189019390

   I will merge the pr


-- 
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@flink.apache.org

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


[GitHub] [flink] godfreyhe commented on a diff in pull request #20242: [FLINK-28490][sql-parser] Introduce `ANALYZE TABLE` syntax in sql parser

Posted by GitBox <gi...@apache.org>.
godfreyhe commented on code in PR #20242:
URL: https://github.com/apache/flink/pull/20242#discussion_r923147131


##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionSpecProperty;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = requireNonNull(tableName, "tableName is null");
+        this.partitions = requireNonNull(partitions, "partitions is null");
+        this.columns = requireNonNull(columns, "columns is null");
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /**
+     * Get partition spec as key-value strings, if only partition key is given, the corresponding
+     * value is null.
+     */
+    public LinkedHashMap<String, String> getPartitions() {
+        LinkedHashMap<String, String> ret = new LinkedHashMap<>();
+        for (SqlNode node : partitions.getList()) {
+            SqlPartitionSpecProperty property = (SqlPartitionSpecProperty) node;
+            final String value;
+            if (property.getValue() != null) {

Review Comment:
   good catch



##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAnalyzeTable.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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.flink.sql.parser.ddl;
+
+import org.apache.flink.sql.parser.SqlPartitionSpecProperty;
+
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlSpecialOperator;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+import org.apache.calcite.util.NlsString;
+
+import javax.annotation.Nonnull;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/** ANALYZE TABLE to compute the statistics for a given table. */
+public class SqlAnalyzeTable extends SqlCall {
+    public static final SqlSpecialOperator OPERATOR =
+            new SqlSpecialOperator("ANALYZE TABLE", SqlKind.OTHER_DDL);
+
+    private final SqlIdentifier tableName;
+    private final SqlNodeList partitions;
+    private final SqlNodeList columns;
+    private final boolean allColumns;
+
+    public SqlAnalyzeTable(
+            SqlParserPos pos,
+            SqlIdentifier tableName,
+            SqlNodeList partitions,
+            SqlNodeList columns,
+            boolean allColumns) {
+        super(pos);
+        this.tableName = requireNonNull(tableName, "tableName is null");
+        this.partitions = requireNonNull(partitions, "partitions is null");
+        this.columns = requireNonNull(columns, "columns is null");
+        this.allColumns = allColumns;
+    }
+
+    public String[] fullTableName() {
+        return tableName.names.toArray(new String[0]);
+    }
+
+    /**
+     * Get partition spec as key-value strings, if only partition key is given, the corresponding
+     * value is null.
+     */
+    public LinkedHashMap<String, String> getPartitions() {
+        LinkedHashMap<String, String> ret = new LinkedHashMap<>();
+        for (SqlNode node : partitions.getList()) {
+            SqlPartitionSpecProperty property = (SqlPartitionSpecProperty) node;
+            final String value;
+            if (property.getValue() != null) {
+                value = null;
+            } else {
+                Comparable<?> comparable = SqlLiteral.value(property.getValue());
+                value =
+                        comparable instanceof NlsString
+                                ? ((NlsString) comparable).getValue()
+                                : comparable.toString();
+            }
+
+            ret.put(property.getKey().getSimple(), value);
+        }
+        return ret;
+    }
+
+    public String[] getColumnNames() {

Review Comment:
   I think array is ok



-- 
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@flink.apache.org

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