[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
> yeah, but ideally any table change should trigger a new analyzing just like
You raise a good point. Maybe we should consider how to ensure at least 
table-level consistency during analysis. For example, one way could be to add 
referenced Tables into the Analyzer such that subsequent references will see 
the same Table object (instead of going to the Catalog every time).

Seems relatively easy to do.

Dimitris also has a good point that distinguishing the drop+add case from the 
invalidate case may be hard, but I think the proposed Analyzer change above 
should handle that, right?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
> Isn't that ok? Once you drop+create a table with same name, it isn't essent
I think that behavior is actually desired. If a table with the same name was 
dropped+added it *should* have a different table id.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
> Alex, Bharath, another possibility is that given our asymmetry architecture
Isn't that ok? Once you drop+create a table with same name, it isn't 
essentially the "same" table anymore, for ex: its schema could've changed. IMO 
thats fine. I think this is what Alex meant when he mentioned that drop+create 
should create a new table ID.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

I think we should avoid both forms of id inconsistency: Different tables with 
the same id, or the same table with different ids.

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
Why do we need another map? Don't we already have a map of existing tables that 
we can use to get the current id?

I think it could actually be problematic to only rely on the TTableName for 
identification. The user could drop+add the same table and I think in that case 
a new id should be assigned. Thoughts?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes