[WIP] Supporting CTAS queries for Hive to Spark query translations #324

nimesh1601 · 2022-10-28T17:02:37Z

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/ParseTreeBuilder.java

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

wmoustafa · 2022-10-31T23:53:54Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+        if(tableRowFormat != null){
+            writer.keyword("ROW FORMAT DELIMITED FIELDS TERMINATED BY");


The condition sounds loose. See all options of the row format: https://cwiki.apache.org/confluence/display/hive/languagemanual+ddl#LanguageManualDDL-RowFormats&SerDe and https://spark.apache.org/docs/latest/sql-ref-syntax-hive-format.html.

It is okay to support only ROW FORMAT DELIMITED FIELDS TERMINATED BY, but the condition should reflect that.

Are you suggesting having a conditional check if entered values for the row format are correct or something else?

I am saying ROW FORMAT implies a few things (e.g., either SERDE or DELIMITED). Somehow you take tableRowFormat to reflect a lot of things together: ROW FORMAT DELIMITED FIELDS TERMINATED BY.

wmoustafa · 2022-10-31T23:57:10Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+    private final @Nullable SqlNode tableSerializer;
+    private final @Nullable SqlNodeList tableFileFormat;
+    private final @Nullable SqlCharStringLiteral tableRowFormat;


Using both serializer and row format is confusing since serializer/deserializer is folded under row format. ROW FORMAT SERDE.

Also, when referencing serializer, can we just use serDe?
You can also remove the table prefix since it is already implied.

Are you suggesting using a separate custom SqlNode for RowFormat which will contain information like serDe, fieldDelim, colDelim, etc? I think having a custom SqlNode will later help in supporting other syntaxes of row formats in a better way.

I am saying ROW FORMAT concept encompasses both SERDE and DELIMITED concepts/keywords. The code structure should reflect the syntax structure.

wmoustafa · 2022-11-01T00:26:09Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+        if(tableFileFormat != null){
+            if(tableFileFormat.size() == 1){
+                writer.keyword("STORED AS");
+                tableFileFormat.get(0).unparse(writer, 0, 0);
+                writer.newlineAndIndent();
+            } else {


Similar to the above, can we make the conditions stricter?

Similarly, are you suggesting to have a check, if the file format is valid, like ORC, Parquet, etc? I think unparsing logic shouldn't worry about these.

wmoustafa · 2022-11-01T00:28:15Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+import java.util.List;
+import java.util.Objects;
+
+public class SqlCreateTable extends SqlCreate {


Could you please depict the SqlNode tree in the comment, or describe what each node is supposed to have? Please see these two examples [1, 2].

Let us get back to this, but it might depend on the discussion above.

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/ParseTreeBuilder.java

wmoustafa · 2022-11-01T00:56:57Z

...-service/src/main/java/com/linkedin/coral/coralservice/controller/TranslationController.java

@@ -64,7 +64,7 @@ public ResponseEntity translate(@RequestBody TranslateRequestBody translateReque
    else if (fromLanguage.equalsIgnoreCase("hive")) {
      // To Spark
      if (toLanguage.equalsIgnoreCase("spark")) {
-        translatedSql = translateHiveToSpark(query);


What is the reason for this change? @ljfgem FYI.

wmoustafa · 2022-11-01T00:58:39Z

coral-service/src/main/java/com/linkedin/coral/coralservice/utils/TranslationUtils.java

@@ -33,4 +36,11 @@ public static String translateHiveToSpark(String query) {
    CoralSpark coralSpark = CoralSpark.create(relNode);
    return coralSpark.getSparkSql();
  }
+
+  public static String translateHiveQueryToSparkSql(String query){


Will get back to this after resolving the above.

wmoustafa · 2022-11-02T04:27:55Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){
+    SparkRelInfo sparkRelInfo;
+    //apply RelNode transformations for sqlNode eligible for transformation.
+    if(sqlNode instanceof SqlCreate) {


How about adding an interface SqlCommand, and have SqlCreateTable extend SqlCreate and implement SqlCommand? SqlCommand defines getSelectQueries() and setSelectQueries(). Those are just list versions of what you have now in SqlCreateTable.

yeah, this sounds good

I feel that getSelectQueries() and setSelectQueries() will be used for CREATE TABLE/VIEW....AS. If there's no other use case, can we rename SqlCommand -> SqlCreateCommand?

Not sure, but I think commands like INSERT OVERWRITE TABLE tmp select * FROM foo, might also leverage this

wmoustafa · 2022-11-02T04:50:23Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *
+   * @return [[CoralSparkInfo]]
+   */
+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){


Note that we are assuming this is the Coral IR SqlNode. Assuming that Coral IR SqlNode has array indexes start from 1 (see standardizeRel()), I am pretty sure the array index in this tree will start from 0. For consistency, somehow we need a SELECT node here that:
1- is aSqlNode
2- has array indexes starting from 1.
I think this implies that the caller to this method must have taken the query already, converted the SELECT SqlNode to RelNode then back to SELECT SqlNode. The former SELECT will have array indexes start from 0, and the latter will have them start from 1. What remains after the caller has done this step is that the latter Coral IR SqlNode is simply converted to Spark SQL string (using the Spark dialect and after applying theSparkSqlRewriter (on the latter SqlNode)), without having to go through RelNode here.
The caller can call the RelNode-based create on the obtained RelNode in the caller. It is best to code this logic in a test case. @aastha25 @ljfgem FYI.

Yes, currently SqlNode given as input to this function has 0-based indexing. Now as you have mentioned that we need a SELECTnode here should have array indexes starting from 1, so it means all RelNodebased transformations, would have been done by the caller of this function. It means the caller should be able to execute the below transformations on theSELECT `node before calling this new create function.

if(sqlNode instanceof SqlCommand) { SqlNode selectNode = ((SqlCreateTable) sqlNode).getSelectQuery(); sparkRelInfo = IRRelToSparkRelTransformer.transform(convertor.apply(selectNode)); selectNode = new CoralRelToSqlNodeConverter().convert(sparkRelInfo.getSparkRelNode()); ((SqlCreateTable) sqlNode).setSelectQuery(selectNode); } else { sparkRelInfo = IRRelToSparkRelTransformer.transform(convertor.apply(sqlNode)); sqlNode = new CoralRelToSqlNodeConverter().convert(sparkRelInfo.getSparkRelNode()); }

Should we create a separate function in CoralSpark to handle this SqlNode->RelNode->SqlNode transformation?
I don't think, we can call RelNode-based create as currently, it will convert RelNode to SQL directly.

in the translation path:
Language1 (hiveSQL) --> CoralSqlNode (LHS) --> CoralRelNode --> CoralSqlNode (RHS) --> Language 2 (spark SQL)

The coral-spark class is responsible for generating sparkSQL from CoralIR. It currently accepts the CoralRelNode and applies the RHS side of transformations to generate a spark-compatible SQL. In your current implementation of create(..), you are passing CoralSqlNode (LHS) and using convertor.apply(selectNode) to generate CoralRelNode. Could you please refactor to apply the LHS side of transformations outside this class and then invoke existing method create(RelNode irRelNode){..} to generate the updated selectNode?

@aastha25 do you mean the entry point of CTAS statement translation should be in another class? I think it's fine to put it in CoralSpark. And looks like create(RelNode irRelNode) can't generate updated selectNode, it would return Spark SQL directly.

@nimesh1601 could you summarize the offline discussion/agreement here?

aastha25

aastha25 · 2022-11-02T23:01:02Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *
+   * @return [[CoralSparkInfo]]
+   */
+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){


If the argument sqlNode is of type SqlCreate, the query's semantics are not getting validated. For example:
for a table foo with multiple columns, if there are input queries like create table t1 as select * as col0 from foo or create table t1 (a int) as select * from foo, I'm not sure if Coral will throw an exception right now.

The validation work can be incremental, but maybe we can setup a class like ddlSqlValidator in coral-common / coral-hive modules which can validate the different types of ddl statements and invoke it here.

We should validate that the rowtype is compatible for SqlCreateTable.

Queries like create table t1 as select * as col0 from foo will fail query parsing, but yes, we would need validation for cases like create table t1 (a int) as select * from foo. Do we have anything similar for select queries?

for select queries, we use the hiveSqlValidator to validate the CoralSqlNode and then generate the relNode representation. This is done here.

if create DDL (and other DDL) are not flowing through this translation path, we should validate them separately.

@aastha25 I do not think we should create two parallel paths. Can we unify them using common APIs?

aastha25 · 2022-11-02T23:02:03Z

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/ParseTreeBuilder.java

+          break;
+      }
+    }
+    return new SqlCreateTable(ZERO, false, ctOptions.ifNotExists != null ? ctOptions.ifNotExists : false, ctOptions.name,


Can we introduce a utility class like calcite does here https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql/ddl/SqlDdlNodes.java to manage all ddl sqlNode creation?

@aastha25 Why does DDL SqlNode creation warrant a separate path? Why cannot it be done through generic SqlNode generation?

aastha25 · 2022-11-02T23:10:12Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){
+    SparkRelInfo sparkRelInfo;
+    //apply RelNode transformations for sqlNode eligible for transformation.
+    if(sqlNode instanceof SqlCreate) {


I feel that getSelectQueries() and setSelectQueries() will be used for CREATE TABLE/VIEW....AS. If there's no other use case, can we rename SqlCommand -> SqlCreateCommand?

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

ljfgem

ljfgem · 2022-11-07T01:32:41Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+    if(sqlNode instanceof SqlCommand) {
+      SqlNode selectNode = ((SqlCreateTable) sqlNode).getSelectQuery();


Why do we cast sqlNode to SqlCreateTable if sqlNode instanceof SqlCommand? Will there be more classes implementing SqlCommand?

ljfgem · 2022-11-07T02:26:30Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *
+   * @return [[CoralSparkInfo]]
+   */
+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){


@aastha25 do you mean the entry point of CTAS statement translation should be in another class? I think it's fine to put it in CoralSpark. And looks like create(RelNode irRelNode) can't generate updated selectNode, it would return Spark SQL directly.

ljfgem · 2022-11-07T02:32:20Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   * Internally Appropriate parts of Sql RelNode is converted to Spark RelNode, Spark RelNode is converted back
+   * to SqlNode and SqlNode to SparkSQL.


Not following Internally Appropriate parts of Sql RelNode, could you clarify it?

ljfgem · 2022-11-07T02:35:46Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *  2) Base tables
+   *  3) Spark UDF information objects, ie. List of {@link SparkUDFInfo}
+   *
+   * @param sqlNode CoralNode which will be translated to SparkSQL.


We need to clarify sqlNode here, it's CoralSqlNode (LHS) in the translation chain Language1 (hiveSQL) --> CoralSqlNode (LHS) --> CoralRelNode --> CoralSqlNode (RHS) --> Language 2 (spark SQL), which is the SqlNode parsed from the source SQL.

ljfgem · 2022-11-07T02:37:41Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *  3) Spark UDF information objects, ie. List of {@link SparkUDFInfo}
+   *
+   * @param sqlNode CoralNode which will be translated to SparkSQL.
+   * @param convertor Functional Interface to convert SqlNode to appropriate RelNode


I think appropriate RelNode can be replaced by CoralRelNode.

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/AbstractASTVisitor.java

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/ParseTreeBuilder.java

aastha25

aastha25 · 2022-11-15T18:06:44Z

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/HiveToRelConverter.java

@@ -52,6 +54,7 @@ public class HiveToRelConverter extends ToRelConverter {
  // The validator must be reused
  SqlValidator sqlValidator = new HiveSqlValidator(getOperatorTable(), getCalciteCatalogReader(),
      ((JavaTypeFactory) getRelBuilder().getTypeFactory()), HIVE_SQL);
+  DdlSqlValidator ddlSqlValidator = new HiveDdlSqlValidator();


could you also make this private final?

aastha25 · 2022-11-16T18:19:08Z

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/validators/HiveDdlSqlValidator.java

+  }
+
+  private void validateCreateTable(SqlNode sqlNode) {
+    //Todo need to add appropriate validations


is this going to be implemented as part of this PR?

aastha25 · 2022-11-16T19:10:45Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *
+   * @return [[SqlNode]]
+   */
+  public static SqlNode getCoralSqlNode(RelNode irRelNode) {


I'm unclear why this is needed.

CoralSpark API should take Coral intermediate representation as input and return Spark SQL / SparkSQLNode.

You could create a

String convert(SqlCommand sqlCommand, RelNode coralRelNode) { // generate CoralSqlNode (RHS) for coralRelNode // set the updated CoralSqlNode (RHS) in sqlCommand // send this (combined) SqlNode to CoralSqlNodeToSparkSqlNodeConverter and then SparkSqlRewriter }

aastha25 · 2022-11-16T19:27:19Z

coral-service/src/main/java/com/linkedin/coral/coralservice/utils/TranslationUtils.java

-    RelNode relNode = new HiveToRelConverter(hiveMetastoreClient).convertSql(query);
-    CoralSpark coralSpark = CoralSpark.create(relNode);
-    return coralSpark.getSparkSql();
+    HiveToRelConverter hiveToRelConverter = new HiveToRelConverter(hiveMetastoreClient);


(Pl read the comment on CoralSpark.getCoralSqlNode(..) before this comment)

I think this class could use a bit of refactoring.
For SqlNode sqlNode = hiveToRelConverter.toSqlNode(query) : hiveToRelConverter could return a coralSqlNode. Currently, on the LHS side, hiveSqlNode is equivalent CoralSqlNode (LHS).

Inside the if condition, you could extract CoralSelectSqlNode from coralSqlNode, and hand it over first to hiveToRelConverter to generate CoralRelNode. And then hand over sqlCommand + CoralRelNode to CoralSpark.

wmoustafa · 2022-12-04T20:50:48Z

coral-common/src/main/java/com/linkedin/coral/common/ToRelConverter.java

    RelRoot root = getSqlToRelConverter().convertQuery(sqlNode, true, true);
+


Unnecessary empty line?

wmoustafa · 2022-12-04T21:01:27Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+        if(tableRowFormat != null){
+            writer.keyword("ROW FORMAT DELIMITED FIELDS TERMINATED BY");


I am saying ROW FORMAT implies a few things (e.g., either SERDE or DELIMITED). Somehow you take tableRowFormat to reflect a lot of things together: ROW FORMAT DELIMITED FIELDS TERMINATED BY.

wmoustafa · 2022-12-04T21:02:57Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+    private final @Nullable SqlNode tableSerializer;
+    private final @Nullable SqlNodeList tableFileFormat;
+    private final @Nullable SqlCharStringLiteral tableRowFormat;


I am saying ROW FORMAT concept encompasses both SERDE and DELIMITED concepts/keywords. The code structure should reflect the syntax structure.

wmoustafa · 2022-12-04T21:03:23Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/SqlCreateTable.java

+import java.util.List;
+import java.util.Objects;
+
+public class SqlCreateTable extends SqlCreate {


Let us get back to this, but it might depend on the discussion above.

wmoustafa · 2022-12-04T21:04:17Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *
+   * @return [[CoralSparkInfo]]
+   */
+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){


@nimesh1601 could you summarize the offline discussion/agreement here?

wmoustafa · 2022-12-04T21:05:23Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+   *
+   * @return [[CoralSparkInfo]]
+   */
+  public static CoralSpark create(SqlNode sqlNode, Function<SqlNode, RelNode> convertor){


@aastha25 I do not think we should create two parallel paths. Can we unify them using common APIs?

wmoustafa · 2022-12-04T21:08:01Z

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/ParseTreeBuilder.java

+          break;
+      }
+    }
+    return new SqlCreateTable(ZERO, false, ctOptions.ifNotExists != null ? ctOptions.ifNotExists : false, ctOptions.name,


@aastha25 Why does DDL SqlNode creation warrant a separate path? Why cannot it be done through generic SqlNode generation?

wmoustafa · 2022-12-04T21:11:24Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/DdlSqlValidator.java

+public interface DdlSqlValidator {
+
+  void validate(SqlNode ddlSqlNode);
+}


Who is supposed to call this (external calling code or internally inside some Coral step)? How can we unify it with the existing pipeline for validation? @aastha25 please let me know your thoughts.

wmoustafa · 2022-12-04T21:12:41Z

coral-common/src/main/java/com/linkedin/coral/common/calcite/sql/util/SqlDdlNodes.java

+public class SqlDdlNodes {
+
+  /** Creates a CREATE TABLE. */
+  public static SqlCreateTable createTable(SqlParserPos pos, boolean replace, boolean ifNotExists, SqlIdentifier name,
+      SqlNodeList columnList, SqlNode query, SqlNode tableSerializer, SqlNodeList tableFileFormat,
+      SqlCharStringLiteral tableRowFormat) {
+    return new SqlCreateTable(pos, replace, ifNotExists, name, columnList, query, tableSerializer, tableFileFormat,
+        tableRowFormat);
+  }


Sounds redundant? @aastha25

wmoustafa · 2022-12-04T21:25:44Z

coral-spark/src/main/java/com/linkedin/coral/spark/CoralSpark.java

+  /**
+   * Users use this function to get CoralSqlNode from CoralRelNode
+   * This should be used when user need to get CoralSqlNode from CoralRelNode by applying
+   * spark specific transformations on CoralRelNode
+   * with Coral-schema output schema
+   *
+   * @return [[SqlNode]]
+   */


Java doc needs to be more elaborate, and reflect the relationship with existing methods. @aastha25 could help with that.

wmoustafa reviewed Oct 31, 2022

View reviewed changes

coral-hive/src/main/java/com/linkedin/coral/hive/hive2rel/parsetree/ParseTreeBuilder.java Outdated Show resolved Hide resolved

nimesh1601 requested a review from wmoustafa November 1, 2022 05:32

wmoustafa reviewed Nov 1, 2022

View reviewed changes

wmoustafa reviewed Nov 2, 2022

View reviewed changes

aastha25 reviewed Nov 2, 2022

View reviewed changes

nimesh1601 added 8 commits November 3, 2022 16:04

Adding SqlCreate Table sql node

70ecd88

Reafctor code to support create table

276ff15

Adding translation logic Coral Spark and adding node of interest logic

75e8478

merge master

b857785

adding more fields to support different variations of create table as…

3ae48d3

… select syntax

fixing ide lint unnncessary changes

a5a2079

fixing ide lint unnncessary changes

d76b2c6

write UT for parse tree builder CTAS

51b5451

nimesh1601 force-pushed the createTableSupport branch from aafad66 to 51b5451 Compare November 3, 2022 13:58

nimesh1601 requested a review from wmoustafa November 3, 2022 14:38

ljfgem reviewed Nov 7, 2022

View reviewed changes

Refactoring code Coral Spark, add Uts and add java doc comments

ce2df6c

nimesh1601 requested review from aastha25 and removed request for wmoustafa November 14, 2022 14:38

nimesh1601 added 2 commits November 15, 2022 22:22

removing unnecessary functions

7027640

add basic uts

4ccd1ce

aastha25 reviewed Nov 16, 2022

View reviewed changes

wmoustafa reviewed Dec 4, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Supporting CTAS queries for Hive to Spark query translations #324

[WIP] Supporting CTAS queries for Hive to Spark query translations #324

		if(tableRowFormat != null){
		writer.keyword("ROW FORMAT DELIMITED FIELDS TERMINATED BY");

		if(sqlNode instanceof SqlCommand) {
		SqlNode selectNode = ((SqlCreateTable) sqlNode).getSelectQuery();

		* Internally Appropriate parts of Sql RelNode is converted to Spark RelNode, Spark RelNode is converted back
		* to SqlNode and SqlNode to SparkSQL.

		RelRoot root = getSqlToRelConverter().convertQuery(sqlNode, true, true);

[WIP] Supporting CTAS queries for Hive to Spark query translations #324

Are you sure you want to change the base?

[WIP] Supporting CTAS queries for Hive to Spark query translations #324

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment