Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17727 )

Change subject: IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17727/5/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/17727/5/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java@85
PS5, Line 85:           ((FeFsTable) 
tbl).getMetaStoreTable().getSd().getSerdeInfo().getSerializationLib());
> line too long (94 > 90)
We can extract "((FeFsTable) tbl).getMetaStoreTable().getSd()" into a variable.


http://gerrit.cloudera.org:8080/#/c/17727/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
File 
testdata/workloads/functional-query/queries/QueryTest/show-create-table.test:

http://gerrit.cloudera.org:8080/#/c/17727/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test@24
PS5, Line 24: test1
Is this correct to reuse "test1"? We already create a "test1" table above. 
Maybe we can rename it to "json_test1"


http://gerrit.cloudera.org:8080/#/c/17727/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test@25
PS5, Line 25: id INT
Could you add all supported types?



--
To view, visit http://gerrit.cloudera.org:8080/17727
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Tue, 10 Aug 2021 13:27:59 +0000
Gerrit-HasComments: Yes

Reply via email to