You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sudhanshu Arora (Code Review)" <ge...@cloudera.org> on 2019/04/16 21:27:52 UTC

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Sudhanshu Arora has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13047


Change subject: [acid] Disallow any operation on full acid table.
......................................................................

[acid] Disallow any operation on full acid table.

Added a simple check to disallow any operation on fully acid table.
Testing Done:
   - I still need to figure out how to add a test case for
     this.
   - Executed the following sql on mini-cluster
     >CREATE TABLE tm (a int, b int) TBLPROPERTIES
     >('transactional'='true',
     >'transactional_properties'='insert_only');

    >select * from tm;

    >CREATE TABLE tm2 (a int, b int) stored as orc TBLPROPERTIES
    >('transactional'='true');

    >Select * from tm2;
    The above select failed with Analysis exception.

Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
---
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
1 file changed, 20 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/13047/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 1
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13047 )

Change subject: [acid] Disallow any operation on full acid table.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13047/1/fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
File fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java:

http://gerrit.cloudera.org:8080/#/c/13047/1/fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java@86
PS1, Line 86:       if (tableTransactionalProp == null || !tableTransactionalProp.equalsIgnoreCase("insert_only")) {
line too long (102 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 1
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Apr 2019 21:28:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13047 )

Change subject: [acid] Disallow any operation on full acid table.
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2863/ : 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/13047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 2
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Apr 2019 20:03:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13047 )

Change subject: [acid] Disallow any operation on full acid table.
......................................................................


Patch Set 2:

Can you file a JIRA and include it in the commit message? This is a significant enough change that we'd want to have an associated JIRA. It's fine if it's an umbrella JIRA that is associated with multiple commits or whatever the most convenient way to organise it for you is.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 2
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Apr 2019 20:02:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Posted by "Sudhanshu Arora (Code Review)" <ge...@cloudera.org>.
Sudhanshu Arora has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13047 )

Change subject: [acid] Disallow any operation on full acid table.
......................................................................

[acid] Disallow any operation on full acid table.

Added a simple check to disallow any operation on fully acid table.
Testing Done:
   - I still need to figure out how to add a test case for
     this.
   - Executed the following sql on mini-cluster
     >CREATE TABLE tm (a int, b int) TBLPROPERTIES
     >('transactional'='true',
     >'transactional_properties'='insert_only');

    >select * from tm;

    >CREATE TABLE tm2 (a int, b int) stored as orc TBLPROPERTIES
    >('transactional'='true');

    >Select * from tm2;
    The above select failed with Analysis exception.

Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
---
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
1 file changed, 21 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/13047/2
-- 
To view, visit http://gerrit.cloudera.org:8080/13047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 2
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Posted by "Sudhanshu Arora (Code Review)" <ge...@cloudera.org>.
Sudhanshu Arora has abandoned this change. ( http://gerrit.cloudera.org:8080/13047 )

Change subject: [acid] Disallow any operation on full acid table.
......................................................................


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 2
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] [acid] Disallow any operation on full acid table.

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13047 )

Change subject: [acid] Disallow any operation on full acid table.
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13047/2/fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
File fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java:

http://gerrit.cloudera.org:8080/#/c/13047/2/fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java@80
PS2, Line 80: transactional
Can we change these strings to constants?
transactional, transactional_properties, insert_only


http://gerrit.cloudera.org:8080/#/c/13047/2/fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java@84
PS2, Line 84: tableIsTransactional != null && tableIsTransactional.equalsIgnoreCase("true")
nit, probably cleaner if we replace this by Boolean.parseBoolean(tableIsTransactional)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb92e5b691bf192980f2cc7d68c491bb1e8455ac
Gerrit-Change-Number: 13047
Gerrit-PatchSet: 2
Gerrit-Owner: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Apr 2019 19:42:48 +0000
Gerrit-HasComments: Yes