Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-07 Thread Hari Sankar Sivarama Subramaniyan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/
---

(Updated May 7, 2016, 9:34 p.m.)


Review request for hive and Ashutosh Chauhan.


Changes
---

Integrated changes as per Ashutosh's comments


Repository: hive-git


Description
---

Describe extended table should show the primary keys/foreign keys associated 
with the table


Diffs (updated)
-

  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 9863d6c 
  metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
  metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql dc27afc 
  metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
  metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql d9194ff 
  metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
  metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql a6b783c 
  metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
  metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql d003a16 
  metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
  metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql 43e984c 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
744512f 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 5c49be9 
  metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
3806e28 
  metastore/src/model/package.jdo b40df39 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java c4d3bfb 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java f4a9772 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 75c2dd9 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
 e76fb91 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
 55e1b3b 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 b5dc0b4 
  ql/src/test/queries/clientpositive/create_with_constraints.q 0bb92e4 
  ql/src/test/results/clientpositive/create_with_constraints.q.out 7a7a50a 

Diff: https://reviews.apache.org/r/46671/diff/


Testing
---


Thanks,

Hari Sankar Sivarama Subramaniyan

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-07 Thread Hari Sankar Sivarama Subramaniyan 


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/create_with_constraints.q.out, line 137
> > 
> >
> > I think words Qualified and  child are redundant here.
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> IMO Child is not redundant since this is a Parent-Child relationship. 
> Agree 'Qualified' can be omitted.
> 
> Ashutosh Chauhan wrote:
> I think child is also redundant because its a describe statement on a 
> table, so its obvious that info printed is for columns belonging to this 
> table.

ok.


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 17
> > 
> >
> > describe formatted should also print constraints info. can you add a 
> > test case for that as well.
> > Also for describe extended/formatted dbName.tblName;
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> This required a patch of changes in the MetaDataFormatUtils class. Will 
> upload in the next patch.
> 
> Ashutosh Chauhan wrote:
> For dbName.tblName or for formatted? Either ways, its ok to do it in a 
> follow-up where we also add support for show create table.

Ashutosh, for "show create table" I will followup with a separate jira. The 
rest, I am planning to support it here.


- Hari Sankar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/#review131529
---


On May 5, 2016, 1:16 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46671/
> ---
> 
> (Updated May 5, 2016, 1:16 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Describe extended table should show the primary keys/foreign keys associated 
> with the table
> 
> 
> Diffs
> -
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
>   metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 744512f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> 5c49be9 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
> 3806e28 
>   metastore/src/model/package.jdo b40df39 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java c4d3bfb 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java f4a9772 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
>  75c2dd9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
>  e76fb91 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
>  55e1b3b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
>  b5dc0b4 
>   ql/src/test/queries/clientpositive/create_with_constraints.q 0bb92e4 
>   ql/src/test/results/clientpositive/create_with_constraints.q.out 7a7a50a 
> 
> Diff: https://reviews.apache.org/r/46671/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hari Sankar Sivarama Subramaniyan
> 
>

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-04 Thread Hari Sankar Sivarama Subramaniyan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/
---

(Updated May 5, 2016, 1:16 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
---

Describe extended table should show the primary keys/foreign keys associated 
with the table


Diffs (updated)
-

  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
  metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
  metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
  metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
  metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
  metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
  metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
  metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
  metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
  metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
  metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
744512f 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 5c49be9 
  metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
3806e28 
  metastore/src/model/package.jdo b40df39 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java c4d3bfb 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java f4a9772 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 75c2dd9 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
 e76fb91 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
 55e1b3b 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 b5dc0b4 
  ql/src/test/queries/clientpositive/create_with_constraints.q 0bb92e4 
  ql/src/test/results/clientpositive/create_with_constraints.q.out 7a7a50a 

Diff: https://reviews.apache.org/r/46671/diff/


Testing
---


Thanks,

Hari Sankar Sivarama Subramaniyan

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-04 Thread Ashutosh Chauhan 


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 19
> > 
> >
> > Can you also add tests for describe formatted tblName colName; where 
> > colName is part of constraint. This should print out constraints relevant 
> > for that column.
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> I dont see a reason why this should be allowed. This will make the column 
> details look complicated unnecessarily (for example, when there are multiple 
> foreign keys associated with the table column). Just printing out the partial 
> constraint information wont be of much use to the end user. Also, constraints 
> are not an entity of the column. Rather if the end-user wants to see the 
> constraints, he/she should do it on a table level.
> 
> Ashutosh Chauhan wrote:
> MySQL prints out what kind of constraint given column is a part of like 
> PK,FK,unique etc. Ref: https://dev.mysql.com/doc/refman/5.0/en/explain.html 
> Adding such an info for Hive will be useful.
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> I dont think we can include any option other than PRIMARY KEY since 
> UNIQUE, MUL, etc are out of scope for this jira. Also, FOREIGN KEY cannot be 
> shown because although its technically a key, it is a parent-child constraint 
> relationship with bidirectional mapping (for e.g we can have PK which is also 
> a FK, which one do we show in this scenario ?, also even if we mark a KEY 
> field as a FK, it can be associated with multiple table columns and in this 
> case just showing FK is of not much use to the end user). 
> 
> Below is the snippet from the documentation you have mentioned:
> 
> The Key field indicates whether the column is indexed:
> 
> If Key is empty, the column either is not indexed or is indexed only as a 
> secondary column in a multiple-column, nonunique index.
> 
> If Key is PRI, the column is a PRIMARY KEY or is one of the columns in a 
> multiple-column PRIMARY KEY.
> 
> If Key is UNI, the column is the first column of a UNIQUE index. (A 
> UNIQUE index permits multiple NULL values, but you can tell whether the 
> column permits NULL by checking the Null field.)
> 
> If Key is MUL, the column is the first column of a nonunique index in 
> which multiple occurrences of a given value are permitted within the column.

OK. We can always add it later if needed.


- Ashutosh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/#review131529
---


On May 3, 2016, 3:46 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46671/
> ---
> 
> (Updated May 3, 2016, 3:46 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Describe extended table should show the primary keys/foreign keys associated 
> with the table
> 
> 
> Diffs
> -
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
>   metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 744512f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> f651a13 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
> 3806e28 
>   metastore/src/model/package.jdo b40df39 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 39a1efc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4d9c3d2 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
>  75c2dd9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
>  55e1b3b 
>   
> ql/sr

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-04 Thread Hari Sankar Sivarama Subramaniyan 


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 19
> > 
> >
> > Can you also add tests for describe formatted tblName colName; where 
> > colName is part of constraint. This should print out constraints relevant 
> > for that column.
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> I dont see a reason why this should be allowed. This will make the column 
> details look complicated unnecessarily (for example, when there are multiple 
> foreign keys associated with the table column). Just printing out the partial 
> constraint information wont be of much use to the end user. Also, constraints 
> are not an entity of the column. Rather if the end-user wants to see the 
> constraints, he/she should do it on a table level.
> 
> Ashutosh Chauhan wrote:
> MySQL prints out what kind of constraint given column is a part of like 
> PK,FK,unique etc. Ref: https://dev.mysql.com/doc/refman/5.0/en/explain.html 
> Adding such an info for Hive will be useful.

I dont think we can include any option other than PRIMARY KEY since UNIQUE, 
MUL, etc are out of scope for this jira. Also, FOREIGN KEY cannot be shown 
because although its technically a key, it is a parent-child constraint 
relationship with bidirectional mapping (for e.g we can have PK which is also a 
FK, which one do we show in this scenario ?, also even if we mark a KEY field 
as a FK, it can be associated with multiple table columns and in this case just 
showing FK is of not much use to the end user). 

Below is the snippet from the documentation you have mentioned:

The Key field indicates whether the column is indexed:

If Key is empty, the column either is not indexed or is indexed only as a 
secondary column in a multiple-column, nonunique index.

If Key is PRI, the column is a PRIMARY KEY or is one of the columns in a 
multiple-column PRIMARY KEY.

If Key is UNI, the column is the first column of a UNIQUE index. (A UNIQUE 
index permits multiple NULL values, but you can tell whether the column permits 
NULL by checking the Null field.)

If Key is MUL, the column is the first column of a nonunique index in which 
multiple occurrences of a given value are permitted within the column.


- Hari Sankar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/#review131529
---


On May 3, 2016, 3:46 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46671/
> ---
> 
> (Updated May 3, 2016, 3:46 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Describe extended table should show the primary keys/foreign keys associated 
> with the table
> 
> 
> Diffs
> -
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
>   metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 744512f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> f651a13 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
> 3806e28 
>   metastore/src/model/package.jdo b40df39 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 39a1efc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4d9c3d2 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
>  75c2dd9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
>  55e1b3b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
>  b5dc0b4 
>   ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
>   ql/src/test/res

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-04 Thread Ashutosh Chauhan 


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 19
> > 
> >
> > Can you also add tests for describe formatted tblName colName; where 
> > colName is part of constraint. This should print out constraints relevant 
> > for that column.
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> I dont see a reason why this should be allowed. This will make the column 
> details look complicated unnecessarily (for example, when there are multiple 
> foreign keys associated with the table column). Just printing out the partial 
> constraint information wont be of much use to the end user. Also, constraints 
> are not an entity of the column. Rather if the end-user wants to see the 
> constraints, he/she should do it on a table level.

MySQL prints out what kind of constraint given column is a part of like 
PK,FK,unique etc. Ref: https://dev.mysql.com/doc/refman/5.0/en/explain.html 
Adding such an info for Hive will be useful.


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/create_with_constraints.q.out, line 137
> > 
> >
> > I think words Qualified and  child are redundant here.
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> IMO Child is not redundant since this is a Parent-Child relationship. 
> Agree 'Qualified' can be omitted.

I think child is also redundant because its a describe statement on a table, so 
its obvious that info printed is for columns belonging to this table.


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 17
> > 
> >
> > describe formatted should also print constraints info. can you add a 
> > test case for that as well.
> > Also for describe extended/formatted dbName.tblName;
> 
> Hari Sankar Sivarama Subramaniyan wrote:
> This required a patch of changes in the MetaDataFormatUtils class. Will 
> upload in the next patch.

For dbName.tblName or for formatted? Either ways, its ok to do it in a 
follow-up where we also add support for show create table.


- Ashutosh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/#review131529
---


On May 3, 2016, 3:46 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46671/
> ---
> 
> (Updated May 3, 2016, 3:46 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Describe extended table should show the primary keys/foreign keys associated 
> with the table
> 
> 
> Diffs
> -
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
>   metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 744512f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> f651a13 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
> 3806e28 
>   metastore/src/model/package.jdo b40df39 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 39a1efc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4d9c3d2 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
>  75c2dd9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
>  55e1b3b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
>  b5dc0b4 
>   ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
>   ql/src/te

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-03 Thread Hari Sankar Sivarama Subramaniyan 


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java,
> >  line 119
> > 
> >
> > These words (primaryKeyInfo etc) should be identical across all 
> > formatters.

The contents are the same. primaryKeyInfo is the key of the json entry.


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 17
> > 
> >
> > describe formatted should also print constraints info. can you add a 
> > test case for that as well.
> > Also for describe extended/formatted dbName.tblName;

This required a patch of changes in the MetaDataFormatUtils class. Will upload 
in the next patch.


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/create_with_constraints.q.out, line 137
> > 
> >
> > I think words Qualified and  child are redundant here.

IMO Child is not redundant since this is a Parent-Child relationship. Agree 
'Qualified' can be omitted.


> On May 3, 2016, 5:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/create_with_constraints.q, line 19
> > 
> >
> > Can you also add tests for describe formatted tblName colName; where 
> > colName is part of constraint. This should print out constraints relevant 
> > for that column.

I dont see a reason why this should be allowed. This will make the column 
details look complicated unnecessarily (for example, when there are multiple 
foreign keys associated with the table column). Just printing out the partial 
constraint information wont be of much use to the end user. Also, constraints 
are not an entity of the column. Rather if the end-user wants to see the 
constraints, he/she should do it on a table level.


- Hari Sankar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/#review131529
---


On May 3, 2016, 3:46 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46671/
> ---
> 
> (Updated May 3, 2016, 3:46 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Describe extended table should show the primary keys/foreign keys associated 
> with the table
> 
> 
> Diffs
> -
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
>   metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 744512f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> f651a13 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
> 3806e28 
>   metastore/src/model/package.jdo b40df39 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 39a1efc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4d9c3d2 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
>  75c2dd9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
>  55e1b3b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
>  b5dc0b4 
>   ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
>   ql/src/test/results/clientpositive/create_with_constraints.q.out 5cf8d83 
> 
> Diff: https://reviews.apache.org/r/46671/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hari Sankar Sivarama Subramaniyan
> 
>

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-03 Thread Ashutosh Chauhan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/#review131529
---




metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 3284)


Should this be equalsIgnoreCase() ?



ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java (line 30)


Please add javadocs for this class.



ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java (line 29)


Please add javadoc for this class.



ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 (line 119)


These words (primaryKeyInfo etc) should be identical across all formatters.



ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 (line 169)


Detailed & Information are redundant. Just use Constraints



ql/src/test/queries/clientpositive/create_with_constraints.q (line 17)


describe formatted should also print constraints info. can you add a test 
case for that as well.
Also for describe extended/formatted dbName.tblName;



ql/src/test/queries/clientpositive/create_with_constraints.q (line 19)


Can you also add tests for describe formatted tblName colName; where 
colName is part of constraint. This should print out constraints relevant for 
that column.



ql/src/test/results/clientpositive/create_with_constraints.q.out (line 116)


There should be line break after Constraints Info



ql/src/test/results/clientpositive/create_with_constraints.q.out (line 137)


I think words Qualified and  child are redundant here.


Schema changes look good. Other code related comments.

- Ashutosh Chauhan


On May 3, 2016, 3:46 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46671/
> ---
> 
> (Updated May 3, 2016, 3:46 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Describe extended table should show the primary keys/foreign keys associated 
> with the table
> 
> 
> Diffs
> -
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
>   metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> 744512f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> f651a13 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
> 3806e28 
>   metastore/src/model/package.jdo b40df39 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 39a1efc 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4d9c3d2 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
>  75c2dd9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
>  55e1b3b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
>  b5dc0b4 
>   ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
>   ql/src/test/results/clientpositive/create_with_constraints.q.out 5cf8d83 
> 
> Diff: https://reviews.apache.org/r/46671/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hari Sankar Sivarama Subramaniyan
> 
>

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-05-02 Thread Hari Sankar Sivarama Subramaniyan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/
---

(Updated May 3, 2016, 3:46 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
---

Describe extended table should show the primary keys/foreign keys associated 
with the table


Diffs (updated)
-

  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
  metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
  metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
  metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
  metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
  metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
  metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
  metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
  metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
  metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
  metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
744512f 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java f651a13 
  metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
3806e28 
  metastore/src/model/package.jdo b40df39 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 39a1efc 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4d9c3d2 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 75c2dd9 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
 55e1b3b 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 b5dc0b4 
  ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
  ql/src/test/results/clientpositive/create_with_constraints.q.out 5cf8d83 

Diff: https://reviews.apache.org/r/46671/diff/


Testing
---


Thanks,

Hari Sankar Sivarama Subramaniyan

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-04-27 Thread Hari Sankar Sivarama Subramaniyan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/
---

(Updated April 28, 2016, 1:25 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
---

Describe extended table should show the primary keys/foreign keys associated 
with the table


Diffs (updated)
-

  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
  metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql b062c56 
  metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 1d00499 
  metastore/scripts/upgrade/mssql/019-HIVE-13076.mssql.sql 00ddb73 
  metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql 2d9cf76 
  metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql c9a5e1d 
  metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 466e950 
  metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql baf855c 
  metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql f57e588 
  metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql ec1fb48 
  metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql e209489 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
744512f 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java f651a13 
  metastore/src/model/org/apache/hadoop/hive/metastore/model/MConstraint.java 
3806e28 
  metastore/src/model/package.jdo b40df39 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cbeb361 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java ab165f1 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 75c2dd9 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
 55e1b3b 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 b5dc0b4 
  ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
  ql/src/test/results/clientpositive/create_with_constraints.q.out 5cf8d83 

Diff: https://reviews.apache.org/r/46671/diff/


Testing
---


Thanks,

Hari Sankar Sivarama Subramaniyan

Re: Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-04-25 Thread Hari Sankar Sivarama Subramaniyan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/
---

(Updated April 26, 2016, 3:06 a.m.)


Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
---

Describe extended table should show the primary keys/foreign keys associated 
with the table


Diffs (updated)
-

  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
744512f 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cbeb361 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java ab165f1 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 75c2dd9 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
 55e1b3b 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 b5dc0b4 
  ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
  ql/src/test/results/clientpositive/create_with_constraints.q.out 5cf8d83 

Diff: https://reviews.apache.org/r/46671/diff/


Testing
---


Thanks,

Hari Sankar Sivarama Subramaniyan

Review Request 46671: HIVE-13598 : Describe extended table should show the primary keys/foreign keys associated with the table

2016-04-25 Thread Hari Sankar Sivarama Subramaniyan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46671/
---

Review request for hive and Ashutosh Chauhan.


Repository: hive-git


Description
---

Describe extended table should show the primary keys/foreign keys associated 
with the table


Diffs
-

  itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java a6e8efa 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
744512f 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cbeb361 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/ForeignKeyInfo.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java ab165f1 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/PrimaryKeyInfo.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java
 75c2dd9 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java
 55e1b3b 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java
 b5dc0b4 
  ql/src/test/queries/clientpositive/create_with_constraints.q eef0c64 
  ql/src/test/results/clientpositive/create_with_constraints.q.out 5cf8d83 

Diff: https://reviews.apache.org/r/46671/diff/


Testing
---


Thanks,

Hari Sankar Sivarama Subramaniyan