[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table>

You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2021/07/26 07:11:05 UTC

[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table stored as JSONFILE - Show Create Table

- Describe

shikha.asrani10@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17727


Change subject: Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the  Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table> - Describe <Table>
......................................................................

Frontend changes to enable 'stored as JSONFILE'
This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/jflex/sql-scanner.flex
6 files changed, 40 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/2
-- 
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: newchange
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>

[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table

stored as JSONFILE - Show Create Table

- Describe

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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

Change subject: Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the  Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table> - Describe <Table>
......................................................................


Patch Set 2:

(2 comments)

We can add FE tests in AnalyzeDDLTest#TestCreateTable, e.g. https://github.com/apache/impala/blob/59d32853ee42886ae683aac95a8be7f9c89b8eb7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java#L2571-L2589

and e2e tests in testdata/workloads/functional-query/queries/QueryTest/show-create-table.test which is used in tests/metadata/test_show_create_table.py::TestShowCreateTable::test_show_create_table.

For tests on DESCRIBE, we can wait for the patch of loading json tables, and then add them in tests/metadata/test_metadata_query_statements.py.

http://gerrit.cloudera.org:8080/#/c/17727/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17727/2//COMMIT_MSG@7
PS2, Line 7: Frontend changes to enable 'stored as JSONFILE'
nit: need a blank line after the title and mention the JIRA id IMPALA-10797 at the beginning.


http://gerrit.cloudera.org:8080/#/c/17727/2/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/17727/2/fe/src/main/cup/sql-parser.cup@299
PS2, Line 299: KW_JSON
To be consistent with Hive, let's use KW_JSONFILE.



-- 
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: 2
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 27 Jul 2021 01:56:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 9: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7389/


-- 
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: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 08:26:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#11).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M be/src/service/query-options-test.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
14 files changed, 118 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/11
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7389/ DRY_RUN=false


-- 
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: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 01:36:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 11: Verified+1


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 16:04:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 11:

> Patch Set 11: Verified-1
> 
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7392/

The failure is unrelated: https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/14571/testReport/junit/query_test.test_fetch/TestFetchAndSpooling/test_rows_sent_counters_protocol__beeswax___exec_option____batch_size___0___num_nodes___0___disable_codegen_rows_threshold___0___disable_codegen___False___abort_on_error___1___exec_single_node_rows_threshold___0____table_format__parquet_none_/

Rerun the job.


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 09:51:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17727/7/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
File fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java:

http://gerrit.cloudera.org:8080/#/c/17727/7/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java@238
PS7, Line 238:  blockSize
nit: keep this unrelative code unchange


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@25
PS5, Line 25: id INT
> Added ORC,AVRO,KUDU. PARQUET, SEQUENCEFILE, TEXT, ICEBERG already have test
Oh sorry, I mean data types, e.g. BIGINT, STRING, TIMESTAMP, etc.

We don't need to add tests for other file formats.



-- 
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: 7
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 00:06:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#4).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
9 files changed, 74 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/4
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table

stored as JSONFILE - Show Create Table

- Describe

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#3).

Change subject: Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the  Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table> - Describe <Table>
......................................................................

Frontend changes to enable 'stored as JSONFILE'
This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
7 files changed, 43 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/3
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7391/ DRY_RUN=true


-- 
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: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 12:08:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

shikha.asrani10@gmail.com 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 9:

(1 comment)

> (2 comments)

Resolved comments.

http://gerrit.cloudera.org:8080/#/c/17727/7/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
File fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java:

http://gerrit.cloudera.org:8080/#/c/17727/7/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java@238
PS7, Line 238: 
> nit: keep this unrelative code unchange
Done



-- 
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: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 01:27:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 13:27:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 11:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7393/ DRY_RUN=false


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 09:53:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17727 )

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Reviewed-on: http://gerrit.cloudera.org:8080/17727
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/query-options-test.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
14 files changed, 118 insertions(+), 28 deletions(-)

Approvals:
  Quanlong Huang: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
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: merged
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 12
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9271/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 6
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 16:59:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

shikha.asrani10@gmail.com 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 4:

(2 comments)

> (2 comments)
 > 
 > We can add FE tests in AnalyzeDDLTest#TestCreateTable, e.g.
 > https://github.com/apache/impala/blob/59d32853ee42886ae683aac95a8be7f9c89b8eb7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java#L2571-L2589
 > 
 > and e2e tests in testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
 > which is used in tests/metadata/test_show_create_table.py::TestShowCreateTable::test_show_create_table.
 > 
 > For tests on DESCRIBE, we can wait for the patch of loading json
 > tables, and then add them in tests/metadata/test_metadata_query_statements.py.

Resolved the comments.

http://gerrit.cloudera.org:8080/#/c/17727/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17727/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'
> nit: need a blank line after the title and mention the JIRA id IMPALA-10797
Done


http://gerrit.cloudera.org:8080/#/c/17727/2/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/17727/2/fe/src/main/cup/sql-parser.cup@299
PS2, Line 299: KW_JSON
> To be consistent with Hive, let's use KW_JSONFILE.
Done



-- 
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: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 23:20:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9262/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 21:12:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#9).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
12 files changed, 115 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/9
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 8: Code-Review+2

(1 comment)

LGTM

http://gerrit.cloudera.org:8080/#/c/17727/8/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
File fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java:

http://gerrit.cloudera.org:8080/#/c/17727/8/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java@238
PS8, Line 238: , blockSize)
nit: this unrelated change comes in again maybe due to your IDE's formatter..



-- 
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: 8
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 01:18:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9287/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 01:47:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7391/


-- 
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: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 18:22:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 11: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7392/


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 07:37:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#6).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
12 files changed, 140 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/6
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table

stored as JSONFILE - Show Create Table

- Describe

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17727 )

Change subject: Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the  Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table> - Describe <Table>
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9164/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 2
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 26 Jul 2021 07:33:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

shikha.asrani10@gmail.com 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:

(4 comments)

> (2 comments)

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@152
PS4, Line 152: Returns the file format associated with the input format class, or null if
             :    * the input format class is not supported.
> I only find two usages of this in AlterTableSetRowFormatStmt#analyze(): htt
Done


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@161
PS4, Line 161:         return HdfsFileFormat.JSON;
             :       }
> We should use String#equals() instead of "==" here. You can add a constant,
Done


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@163
PS4, Line 163:     }
> Could you add a Precondition check above this to make sure input format is 
Done


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@183
PS4, Line 183:   public static HdfsFileFormat fromThrift(THdfsFileFormat thriftFormat) {
> same comment as above
Done



-- 
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 <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 16:17:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#10).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M be/src/service/query-options-test.cc
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
13 files changed, 117 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/10
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9267/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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 <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 11:20:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9288/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 12:00:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@152
PS4, Line 152: 
             :   public static HdfsFileFormat fromHdfsInputFormatClass(String inputFormatClass) {
Is this override still used somewhere? If so, should we replace them with the below override to avoid inconsistence?


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@163
PS4, Line 163:         return HdfsFileFormat.JSON;
Could you add a Precondition check above this to make sure input format is text? i.e.

  Preconditions.checkState(TEXT.inputFormat().equals(inputFormatClass));


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@183
PS4, Line 183:         return HdfsFileFormat.JSON;
same comment as above


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
File fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java:

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java@238
PS4, Line 238:  blockSize
nit: let's don't modify unrelated codes



-- 
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: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 23:16:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#8).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
12 files changed, 116 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/8
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#7).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
12 files changed, 140 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/7
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9290/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 01:34:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table

stored as JSONFILE - Show Create Table

- Describe

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17727 )

Change subject: Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the  Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table> - Describe <Table>
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9250/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 3
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Aug 2021 05:02:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9275/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 7
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 20:52:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 9: Code-Review+2

Thanks for addressing all the comments!


-- 
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: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 01:35:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

shikha.asrani10@gmail.com 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 4:

(1 comment)

> (4 comments)

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@152
PS4, Line 152: 
             :   public static HdfsFileFormat fromHdfsInputFormatClass(String inputFormatClass) {
> Is this override still used somewhere? If so, should we replace them with t
Yes it is being used in few other places.
Should I pass Null serde for places where we don't need to use serde?
OR should I pass down the serde in all cases and use only when the inputFormatClass is Text.



-- 
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: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 23:36:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@152
PS4, Line 152: 
             :   public static HdfsFileFormat fromHdfsInputFormatClass(String inputFormatClass) {
> Yes it is being used in few other places.
I only find two usages of this in AlterTableSetRowFormatStmt#analyze(): https://github.com/apache/impala/blob/b03d18863b31f0f3e66e9fa1f84cc9d625ecce29/fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java#L79-L85

I think we can refactor the first usage to use partition.getFileFormat().name() directly, and refactor the second usage to providing the actual serDe.


http://gerrit.cloudera.org:8080/#/c/17727/4/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@161
PS4, Line 161:     if (serDe != null) {
             :       if (serDe == "org.apache.hadoop.hive.serde2.lazy.JsonSerDe") {
We should use String#equals() instead of "==" here. You can add a constant, e.g. JSON_SERDE, as "org.apache.hadoop.hive.serde2.lazy.JsonSerDe", and then change this to

  if (JSON_SERDE.equals(serDe)) {



-- 
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: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 03:30:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9285/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 8
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 23:44:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the Json File like: - Create Table

stored as JSONFILE - Show Create Table

- Describe

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17727 )

Change subject: Frontend changes to enable 'stored as JSONFILE' This change will allow usage of commands that do not require reading the  Json File like: - Create Table <Table> stored as JSONFILE - Show Create Table <Table> - Describe <Table>
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17727/3/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
File fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java:

http://gerrit.cloudera.org:8080/#/c/17727/3/fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java@231
PS3, Line 231:           HdfsFileFormat.fromJavaClassName(sd.getInputFormat(),sd.getSerdeInfo().getSerializationLib()),
line too long (104 > 90)



-- 
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: 3
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Aug 2021 04:41:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

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 11: Code-Review+2


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 06:29:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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 11:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7392/ DRY_RUN=true


-- 
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: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Aug 2021 01:36:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

shikha.asrani10@gmail.com 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 6:

(3 comments)

> (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:           sd.getInputFormat(), sd.getSerdeInfo().getSerializationLib());
> We can extract "((FeFsTable) tbl).getMetaStoreTable().getSd()" into a varia
Done


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: json_
> Is this correct to reuse "test1"? We already create a "test1" table above. 
Done


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?
Added ORC,AVRO,KUDU. PARQUET, SEQUENCEFILE, TEXT, ICEBERG already have tests in the file.



-- 
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: 6
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 16:42:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17727

to look at the new patch set (#5).

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

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

This change will allow usage of commands that do not require reading the
 Json File like:
- Create Table <Table> stored as JSONFILE
- Show Create Table <Table>
- Describe <Table>

Changes:
- Added JSON as FileFormat to thrift  and HdfsFileFormat.
- Allowing Sql keyword 'jsonfile' and mapping it to JSON format.
- Adding JSON serDe.
- JsonFiles have input format same as TextFile, so we need to use SerDe
library in use to differentiate between the two formats. Overloaded the
functions querying File Format based on input format to consider serDe
library too.
- Added tests for 'Create Table' and 'Show Create Table' commmands

Pending Changes:
- test for Describe command - to be added with backend changes.

Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetRowFormatStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
12 files changed, 76 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/27/17727/5
-- 
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: newpatchset
Gerrit-Change-Id: I5b8cb2f59df3af09902b49d3bdac16c19954b305
Gerrit-Change-Number: 17727
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10797: Frontend changes to enable 'stored as JSONFILE'

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins 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)


http://gerrit.cloudera.org:8080/#/c/17727/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/17727/5/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1724
PS5, Line 1724:     String serDeLib = msTbl.getSd().getSerdeInfo().getSerializationLib();    
line has trailing whitespace


http://gerrit.cloudera.org:8080/#/c/17727/5/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:

http://gerrit.cloudera.org:8080/#/c/17727/5/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@364
PS5, Line 364:     return HdfsFileFormat.fromJavaClassName(inputFormat, serDeLib) == 
line has trailing whitespace



-- 
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 <sh...@gmail.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 10:59:33 +0000
Gerrit-HasComments: Yes