Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8928 )
Change subject: IMPALA-4323: "SET ROW FORMAT" option added to "ALTER TABLE" command ...................................................................... Patch Set 1: (4 comments) Updated patch 1. http://gerrit.cloudera.org:8080/#/c/8928/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8928/1//COMMIT_MSG@7 PS1, Line 7: Added the "SET ROW FORMAT" option to the "ALTER TABLE" command. > Adds "IMPALA-4323: " at the start of the line. Done http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/cup/sql-parser.cup@1003 PS1, Line 1003: | KW_ALTER KW_TABLE table_name:table opt_partition_set:partition KW_SET > Add some unit tests(white & black) into ParserTest.java. See Done http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: http://gerrit.cloudera.org:8080/#/c/8928/3/fe/src/main/cup/sql-parser.cup@1007 PS3, Line 1007: // row_format_val cannot be used as it conflicts with cache_op_val. > Hm, why would it conflict with cache_op_val? Can you post the error message When I dump the grammar, the following two lines seem to be the problem: [128] alter_tbl_stmt ::= KW_ALTER KW_TABLE table_name opt_partition_set KW_SET row_format_val escaped_by_val line_terminator_val [129] alter_tbl_stmt ::= KW_ALTER KW_TABLE table_name opt_partition_set KW_SET cache_op_val Here are the errors: [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Reduce/Reduce conflict found in state #1242 between row_format_val ::= (*) and cache_op_val ::= (*) under symbols: {EOF, SEMICOLON} Resolved in favor of the second production. [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Shift/Reduce conflict found in state #1242 between row_format_val ::= (*) under symbol EOF Resolved in favor of shifting. [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Shift/Reduce conflict found in state #1242 between row_format_val ::= (*) under symbol SEMICOLON Resolved in favor of shifting. [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Shift/Reduce conflict found in state #1242 between cache_op_val ::= (*) under symbol EOF Resolved in favor of shifting. [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Shift/Reduce conflict found in state #1242 between cache_op_val ::= (*) under symbol SEMICOLON Resolved in favor of shifting. [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Shift/Reduce conflict found in state #892 between line_terminator_val ::= (*) and line_terminator_val ::= (*) KW_LINES terminator_val under symbol KW_LINES Resolved in favor of shifting. [WARNING] /home/impdev/Impala/fe/src/main/cup/sql-parser.cup [0:0]: *** Shift/Reduce conflict found in state #890 between escaped_by_val ::= (*) and escaped_by_val ::= (*) KW_ESCAPED KW_BY STRING_LITERAL under symbol KW_ESCAPED Resolved in favor of shifting. http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java: http://gerrit.cloudera.org:8080/#/c/8928/1/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@35 PS1, Line 35: > nit: use space instead of tab Done -- To view, visit http://gerrit.cloudera.org:8080/8928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96e347463504915a6f33932552e4d1f61e9b1154 Gerrit-Change-Number: 8928 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: John Russell <[email protected]> Gerrit-Reviewer: Kim Jin Chul <[email protected]> Gerrit-Comment-Date: Sat, 06 Jan 2018 17:51:16 +0000 Gerrit-HasComments: Yes
