You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by robertamarton <gi...@git.apache.org> on 2015/09/04 19:38:06 UTC

[GitHub] incubator-trafodion pull request: Support for features described i...

GitHub user robertamarton opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/74

    Support for features described in JIRA TRAFODION-19 && TRAFODION-1483

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/robertamarton/incubator-trafodion master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/74.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #74
    
----
commit 2e0b1ddf10efbd09fe027ce3af00aef7ba821271
Author: Roberta Marton <ro...@esgyn.com>
Date:   2015-09-04T17:09:32Z

    JIRA TRAFODION-19 && TRAFODION-1483
    
    JIRA TRAFODION-19:
    
    Made the following changes:
    
    Removed support for locklength from the parser
    Test and fix issues with create table variants
    Made recommended changes as mentioned in the initial delivery
      Rename some of the variable names to make them more informative
      Assume prefix length for HIVE and HBASE tables can be different and verify code works correctly
      Fix commented code to be correct and remove lines no longer needed
      Update error text for messages 1180 to include external table type
    
    Beefed up the new regression test (hive/TEST009)
    Check column list from hive descriptor with external table description and return an error if there is a mismatch
    Add an error if the user specifies a different target schema name than the source
    Allow someone with privileges to drop HIVE exernal schemas
    
    JIRA TRAFODION-1483:
    
    Added support for external HBase tables similar to Hive table support

commit 569045b4fb816e598f5ec1e1bed45894aebf199d
Author: Roberta Marton <ro...@esgyn.com>
Date:   2015-09-04T17:11:13Z

    Merge branch 'master' of github.com:apache/incubator-trafodion

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38961242
  
    --- Diff: core/sql/optimizer/BindRelExpr.cpp ---
    @@ -1569,12 +1569,11 @@ NATable *BindWA::getNATable(CorrName& corrName,
           ((QualifiedName&)(table->getTableName())).setIsVolatile(TRUE);
         }
           
    -  // For now, do not allow access through the Trafodion external name created for
    -  // the HIVE object unless the inDDL flag is set.  inDDL is set for drop 
    -  // table and SHOWDDL statements.  
    -  // TDB - may want to merge the Trafodion version with the HIVE version.
    -  // TDB - similar operation may be needed for external HBase tables
    -  if ((table) && (table->isExternalTable() && (! bindWA->inDDL())))
    +  // For now, don't allow access through the Trafodion external name created for
    +  // native HIVE or HBASE objects unless the allowExternalTables flag is set.  
    +  // allowExternalTables is set for drop table and SHOWDDL statements.  
    +  // TDB - may want to merge the Trafodion version with the native version.
    +  if ((table) && (table->isExternalTable() && (! bindWA->allowExternalTables())))
    --- End diff --
    
    We could open this up and allow the customer to use either name.  The same results should be returned independent of the name.  They are sort of like synonyms. But the way it is coded is that the customer will always use the hive.sch.table or hbase.sch.table name.  They really don't need to worry about the registered (external) name.  
    
    We could open it up for all requests but there will be additional code and testing.  I could create a JIRA to change this or wait and see how people use external tables to see if this is a desired feature.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38949821
  
    --- Diff: core/sql/common/ComMisc.cpp ---
    @@ -99,17 +100,74 @@ NABoolean ComIsTrafodionReservedSchemaName(
       return FALSE;
     }
     
    -// schema names of pattern "_HV ... _" and "_HB_ ... _" are reserved to store
    +// schema names of pattern "_HV_ ... _" and "_HB_ ... _" are reserved to store
     // external hive and hbase tables
     NABoolean ComIsTrafodionExternalSchemaName (
                                         const NAString &schName)
     {
       Int32 len (schName.length());
    +
    +  // check for HIVE
       Int32 prefixLen = sizeof(HIVE_EXT_SCHEMA_PREFIX);
       if (len > prefixLen && 
    -      (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX || 
    -       schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX) && 
    -      schName(len-1) == '_' )
    +     (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
         return TRUE;
    +
    +  // check for HBASE
    +  prefixLen = sizeof(HBASE_EXT_SCHEMA_PREFIX);
    +  if (len > prefixLen && 
    +     (schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
    +    return TRUE;
    +
       return FALSE;
     }
    +
    +// ----------------------------------------------------------------------------
    +// function: ComConvertNativeNameToTrafName
    +//
    +// this fuction converts the native HIVE or HBASE object name into its
    --- End diff --
    
    function? (typo)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38956791
  
    --- Diff: core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---
    @@ -379,84 +387,40 @@ void CmpSeabaseDDL::createSeabaseTableLike(
     }
     
     // ----------------------------------------------------------------------------
    -// Method: createSeabaseTableExternalHive
    +// Method: createSeabaseTableExternal
    --- End diff --
    
    Thanks. This will be a very useful methods during PoCs. Maybe the bulk loader could take a create option and use this method to create an empty Traf table before loading into it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38955476
  
    --- Diff: core/sql/regress/hive/EXPECTED009 ---
    @@ -266,10 +276,301 @@ A            B            C
     *** ERROR[8822] The statement was not prepared.
     
     >>
    +>>set schema "_HV_HIVE_";
    +
    +--- SQL operation complete.
    +>>
    +>>-- all these creates should fail, they are not supported yet
    +>>create table hive_customer like hive.hive.customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".HIVE_CUSTOMER is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable1 like hive.hive.customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE1 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable2 like customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE2 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable3 (a int);
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE3 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>get tables;
    +
    +Tables in Schema TRAFODION._HV_HIVE_
    +====================================
    +
    +CUSTOMER
    +ITEM
    +PROMOTION
    +
    +--- SQL operation complete.
    +>>
    +>>-- test creates with a different default schema
    +>>create schema hive_t009;
    +
    +--- SQL operation complete.
    +>>set schema hive_t009;
    +
    +--- SQL operation complete.
    +>>
    +>>-- these creates fail
    +>>create table hive_customer like hive.hive.customer;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable1 like hive.hive.customer;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create external table seabase.customer like hive.hive.customer;
    +
    +*** ERROR[1180] Trying to create an external HIVE table with a different schema or table name (SEABASE) than the source table (HIVE).  The external schema and table name must be the same as the source.
    +
    +--- SQL operation failed with errors.
    +>>create external table customer1 like hive.hive.customer;
    +
    +*** ERROR[1180] Trying to create an external HIVE table with a different schema or table name (CUSTOMER1) than the source table (CUSTOMER).  The external schema and table name must be the same as the source.
    +
    +--- SQL operation failed with errors.
    +>>create table t009t1 like "_HV_SCH_T009_".t009t1;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create table t009t2 as select * from "_HV_SCH_T009_".t009t2;
    +
    +*** ERROR[4258] Trying to access external table TRAFODION."_HV_SCH_T009_".T009T2 through its external name format. Please use the native table name.
    +
    +*** ERROR[8822] The statement was not prepared.
    +
    +>>
    +>>-- this create succeeds
    +>>create table t009t1 as select * from hive.sch_t009.t009t1;
    --- End diff --
    
    Is t009t1 a Trafodion table or a hive external table (with an internal already created in a different script)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38960453
  
    --- Diff: core/sql/common/ComMisc.cpp ---
    @@ -99,17 +100,74 @@ NABoolean ComIsTrafodionReservedSchemaName(
       return FALSE;
     }
     
    -// schema names of pattern "_HV ... _" and "_HB_ ... _" are reserved to store
    +// schema names of pattern "_HV_ ... _" and "_HB_ ... _" are reserved to store
     // external hive and hbase tables
     NABoolean ComIsTrafodionExternalSchemaName (
                                         const NAString &schName)
     {
       Int32 len (schName.length());
    +
    +  // check for HIVE
       Int32 prefixLen = sizeof(HIVE_EXT_SCHEMA_PREFIX);
       if (len > prefixLen && 
    -      (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX || 
    -       schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX) && 
    -      schName(len-1) == '_' )
    +     (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
         return TRUE;
    +
    +  // check for HBASE
    +  prefixLen = sizeof(HBASE_EXT_SCHEMA_PREFIX);
    +  if (len > prefixLen && 
    +     (schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
    +    return TRUE;
    +
       return FALSE;
     }
    +
    +// ----------------------------------------------------------------------------
    +// function: ComConvertNativeNameToTrafName
    +//
    +// this fuction converts the native HIVE or HBASE object name into its
    +// Trafodion external name format.
    +//
    +// params:
    +//    catalogName - catalog name to identify HBASE or HIVE native table
    +//    schemaName - external name of the HBASE or HIVE schema
    +//    objectName - external name of the HBASE of HIVE table
    +//
    +// If it is not HIVE or HBASE, just return the qualified name
    +// ----------------------------------------------------------------------------
    +NAString ComConvertNativeNameToTrafName ( 
    +  const NAString &catalogName,
    +  const NAString &schemaName,
    +  const NAString &objectName)
    +{
    +  // generate new schema name 
    +  NAString tempSchemaName; 
    +  if (catalogName == HIVE_SYSTEM_CATALOG)
    +    tempSchemaName += HIVE_EXT_SCHEMA_PREFIX;
    +  else if(catalogName == HBASE_SYSTEM_CATALOG)
    +    tempSchemaName += HBASE_EXT_SCHEMA_PREFIX;
    +  else
    +    return catalogName + NAString(".") +
    +           schemaName + NAString(".") +
    +           objectName; 
    +
    +  ComAnsiNamePart externalAnsiName(schemaName, ComAnsiNamePart::EXTERNAL_FORMAT);
    +  tempSchemaName += externalAnsiName.getInternalName();
    +  tempSchemaName.append ("_");
    +
    +  // Catalog name is "TRAFODION"
    +  NAString convertedName (CmpSeabaseDDL::getSystemCatalogStatic());
    +  convertedName += ".";
    +
    +  // append transformed schema name, convert internal name to external format
    +  ComAnsiNamePart internalAnsiName(tempSchemaName, ComAnsiNamePart::INTERNAL_FORMAT);
    +  convertedName += internalAnsiName.getExternalName();
    +
    +  // object  name is appended without change
    --- End diff --
    
    We require the names to be in external format before entering this method.  I am using the ComAnsiNamePart class to convert from external to internal and back.  The constructor calls a method called "copyExternalName" which checks for a valid name.  However,  the constructor ignores the errors.  It looks like the method "isValid" should be called after constructing the object.  In this case, the name will be empty.  
    
    I did run tests with both delimited and non-delimited names so the code is working correctly today.  In the future, we should probably change this code.  We should also look at other places where ComAnsiNamePart is called to make sure a similar issue does not exist.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38962436
  
    --- Diff: core/sql/regress/hive/EXPECTED009 ---
    @@ -266,10 +276,301 @@ A            B            C
     *** ERROR[8822] The statement was not prepared.
     
     >>
    +>>set schema "_HV_HIVE_";
    +
    +--- SQL operation complete.
    +>>
    +>>-- all these creates should fail, they are not supported yet
    +>>create table hive_customer like hive.hive.customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".HIVE_CUSTOMER is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable1 like hive.hive.customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE1 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable2 like customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE2 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable3 (a int);
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE3 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>get tables;
    +
    +Tables in Schema TRAFODION._HV_HIVE_
    +====================================
    +
    +CUSTOMER
    +ITEM
    +PROMOTION
    +
    +--- SQL operation complete.
    +>>
    +>>-- test creates with a different default schema
    +>>create schema hive_t009;
    +
    +--- SQL operation complete.
    +>>set schema hive_t009;
    +
    +--- SQL operation complete.
    +>>
    +>>-- these creates fail
    +>>create table hive_customer like hive.hive.customer;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable1 like hive.hive.customer;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create external table seabase.customer like hive.hive.customer;
    +
    +*** ERROR[1180] Trying to create an external HIVE table with a different schema or table name (SEABASE) than the source table (HIVE).  The external schema and table name must be the same as the source.
    +
    +--- SQL operation failed with errors.
    +>>create external table customer1 like hive.hive.customer;
    +
    +*** ERROR[1180] Trying to create an external HIVE table with a different schema or table name (CUSTOMER1) than the source table (CUSTOMER).  The external schema and table name must be the same as the source.
    +
    +--- SQL operation failed with errors.
    +>>create table t009t1 like "_HV_SCH_T009_".t009t1;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create table t009t2 as select * from "_HV_SCH_T009_".t009t2;
    +
    +*** ERROR[4258] Trying to access external table TRAFODION."_HV_SCH_T009_".T009T2 through its external name format. Please use the native table name.
    +
    +*** ERROR[8822] The statement was not prepared.
    +
    +>>
    +>>-- this create succeeds
    +>>create table t009t1 as select * from hive.sch_t009.t009t1;
    +
    +--- 10 row(s) inserted.
    +>>
    +>>get tables;
    +
    +Tables in Schema TRAFODION.HIVE_T009
    +====================================
    +
    +T009T1
    +
    +--- SQL operation complete.
    +>>drop table t009t1;
    +
    +--- SQL operation complete.
    +>>
     >>drop external table "_HV_HIVE_".customer;
     
     --- SQL operation complete.
     >>drop external table item for hive.hive.item;
     
     --- SQL operation complete.
    +>>
    +>>obey TEST009(test_hive2);
    +>>-- drop data from the hive table and recreate with 4 columns
    +>>-- this causes the external table to be invalid
    +>>
    +>>-- cleanup data from the old table, and create/load data with additional column
    +>>sh regrhadoop.ksh fs -rm   /user/hive/exttables/t009t1/*;
    +>>sh regrhive.ksh -v -f $REGRTSTDIR/TEST009_b.hive.sql;
    +>>
    +>>-- should fail - column mismatch
    +>>select count(*) from hive.sch_t009.t009t1;
    +
    +*** ERROR[3078] The column list for HIVE.SCH_T009.T009T1 does not match its external table representation defined as TRAFODION."_HV_SCH_T009_".T009T1.
    --- End diff --
    
    I actually toyed with this and could not decide whether this was an error or warning.
    
    When I was testing, I discovered that the information I stored in the metadata did not match the information returned in the hive_tbl_desc.  This actually pointed to a couple of errors in the code.  There is also an issue when a mismatch does occur, that an error is returned and you have no idea what the issue is.  I had to add debug code to figure it out.
    
    So this area can be handled better.  I can go either way - error or warning.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38950572
  
    --- Diff: core/sql/optimizer/BindRelExpr.cpp ---
    @@ -1569,12 +1569,11 @@ NATable *BindWA::getNATable(CorrName& corrName,
           ((QualifiedName&)(table->getTableName())).setIsVolatile(TRUE);
         }
           
    -  // For now, do not allow access through the Trafodion external name created for
    -  // the HIVE object unless the inDDL flag is set.  inDDL is set for drop 
    -  // table and SHOWDDL statements.  
    -  // TDB - may want to merge the Trafodion version with the HIVE version.
    -  // TDB - similar operation may be needed for external HBase tables
    -  if ((table) && (table->isExternalTable() && (! bindWA->inDDL())))
    +  // For now, don't allow access through the Trafodion external name created for
    +  // native HIVE or HBASE objects unless the allowExternalTables flag is set.  
    +  // allowExternalTables is set for drop table and SHOWDDL statements.  
    +  // TDB - may want to merge the Trafodion version with the native version.
    +  if ((table) && (table->isExternalTable() && (! bindWA->allowExternalTables())))
    --- End diff --
    
    I have forgotten why we thought this restriction is necessary.
    
    A user registers a Hive/HBase table with us but is not able to use the table with the registered name in DML. Will UPDATE STATs will be on the original name too (and update stats code will find out registered name and use it for Table_uid?). This was the registration concept is almost like an internal implementation idea, at some point could even be done by some utility program for all existing Hive/hBase tables, and the user need not be aware of it?  
    
    I am struggling with a user would want to register a table, if they cannot use it by that name.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38952891
  
    --- Diff: core/sql/optimizer/RelExeUtil.cpp ---
    @@ -3814,23 +3814,21 @@ RelExpr * DDLExpr::bindNode(BindWA *bindWA)
     
           qualObjName_ = dropTableNode->getTableNameAsQualifiedName();
     
    -      // Hive tables can only be specified as external and must be created
    -      // with the FOR clause
    -      if (qualObjName_.isHive())
    -      {
    -        if (dropTableNode->isExternal())
    +      // Drops of Hive and HBase external tables are allowed 
    +      if (qualObjName_.isHive() || (qualObjName_.isHbase()))
             {
    -          isHbase_ = TRUE;
    -          externalTable = TRUE;
    -        }
    -        else
    -        {
    -          *CmpCommon::diags() << DgSqlCode(-4222) << DgString0("DDL");
    -          bindWA->setErrStatus();
    -          return NULL;
    +          if (dropTableNode->isExternal())
    +            {
    +              isHbase_ = TRUE;
    --- End diff --
    
    What does this flag mean? should it be set for a Hive external table too?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/74


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38961379
  
    --- Diff: core/sql/optimizer/RelExeUtil.cpp ---
    @@ -3814,23 +3814,21 @@ RelExpr * DDLExpr::bindNode(BindWA *bindWA)
     
           qualObjName_ = dropTableNode->getTableNameAsQualifiedName();
     
    -      // Hive tables can only be specified as external and must be created
    -      // with the FOR clause
    -      if (qualObjName_.isHive())
    -      {
    -        if (dropTableNode->isExternal())
    +      // Drops of Hive and HBase external tables are allowed 
    +      if (qualObjName_.isHive() || (qualObjName_.isHbase()))
             {
    -          isHbase_ = TRUE;
    -          externalTable = TRUE;
    -        }
    -        else
    -        {
    -          *CmpCommon::diags() << DgSqlCode(-4222) << DgString0("DDL");
    -          bindWA->setErrStatus();
    -          return NULL;
    +          if (dropTableNode->isExternal())
    +            {
    +              isHbase_ = TRUE;
    --- End diff --
    
    isExternal is set when the "external" keyword is used in the drop table syntax.  It is set for both hbase and hive tables.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38962003
  
    --- Diff: core/sql/parser/StmtDDLDrop.cpp ---
    @@ -561,7 +561,8 @@ StmtDDLDropSchema::StmtDDLDropSchema(//const SchemaName & schemaName,
       // If the schema name specified is reserved name, users cannot drop them.
       // They can only be dropped internally.
       if ((! Get_SqlParser_Flags(INTERNAL_QUERY_FROM_EXEUTIL)) &&
    -      (ComIsTrafodionReservedSchemaName(schemaQualName_.getSchemaName())))
    +      (ComIsTrafodionReservedSchemaName(schemaQualName_.getSchemaName())) &&
    +      (!ComIsTrafodionExternalSchemaName(schemaQualName_.getSchemaName())))
    --- End diff --
    
    Sorry, I should have written this cleaner.  
    
    The first check - if the parserflag is set, then continue and don't return an error.
    
    The second check - if the schema name does not begin with an underscore, then continue and don't return an error.
    
    The final check - if the schema name is not an external name, that is begins with "_HV_" or "_HB_", then this is an error.  We don't want to drop tables beginning with the "_".
    
    I added this check so people who have the privilege can drop externally generated schemas.  If this check did not exist, then you have to be DB__ROOT to remove these schemas.  I thought this was a little extreme.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38958590
  
    --- Diff: core/sql/common/ComMisc.cpp ---
    @@ -99,17 +100,74 @@ NABoolean ComIsTrafodionReservedSchemaName(
       return FALSE;
     }
     
    -// schema names of pattern "_HV ... _" and "_HB_ ... _" are reserved to store
    +// schema names of pattern "_HV_ ... _" and "_HB_ ... _" are reserved to store
     // external hive and hbase tables
     NABoolean ComIsTrafodionExternalSchemaName (
                                         const NAString &schName)
     {
       Int32 len (schName.length());
    +
    +  // check for HIVE
       Int32 prefixLen = sizeof(HIVE_EXT_SCHEMA_PREFIX);
       if (len > prefixLen && 
    -      (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX || 
    -       schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX) && 
    -      schName(len-1) == '_' )
    +     (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
         return TRUE;
    +
    +  // check for HBASE
    +  prefixLen = sizeof(HBASE_EXT_SCHEMA_PREFIX);
    +  if (len > prefixLen && 
    +     (schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
    +    return TRUE;
    +
       return FALSE;
     }
    +
    +// ----------------------------------------------------------------------------
    +// function: ComConvertNativeNameToTrafName
    +//
    +// this fuction converts the native HIVE or HBASE object name into its
    --- End diff --
    
    Yep, will fix on my next delivery.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38956114
  
    --- Diff: core/sql/regress/hive/EXPECTED009 ---
    @@ -266,10 +276,301 @@ A            B            C
     *** ERROR[8822] The statement was not prepared.
     
     >>
    +>>set schema "_HV_HIVE_";
    +
    +--- SQL operation complete.
    +>>
    +>>-- all these creates should fail, they are not supported yet
    +>>create table hive_customer like hive.hive.customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".HIVE_CUSTOMER is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable1 like hive.hive.customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE1 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable2 like customer;
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE2 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable3 (a int);
    +
    +*** ERROR[1118] Creating object TRAFODION."_HV_HIVE_".NEWTABLE3 is not allowed in a reserved system schema.
    +
    +--- SQL operation failed with errors.
    +>>get tables;
    +
    +Tables in Schema TRAFODION._HV_HIVE_
    +====================================
    +
    +CUSTOMER
    +ITEM
    +PROMOTION
    +
    +--- SQL operation complete.
    +>>
    +>>-- test creates with a different default schema
    +>>create schema hive_t009;
    +
    +--- SQL operation complete.
    +>>set schema hive_t009;
    +
    +--- SQL operation complete.
    +>>
    +>>-- these creates fail
    +>>create table hive_customer like hive.hive.customer;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create table newtable1 like hive.hive.customer;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create external table seabase.customer like hive.hive.customer;
    +
    +*** ERROR[1180] Trying to create an external HIVE table with a different schema or table name (SEABASE) than the source table (HIVE).  The external schema and table name must be the same as the source.
    +
    +--- SQL operation failed with errors.
    +>>create external table customer1 like hive.hive.customer;
    +
    +*** ERROR[1180] Trying to create an external HIVE table with a different schema or table name (CUSTOMER1) than the source table (CUSTOMER).  The external schema and table name must be the same as the source.
    +
    +--- SQL operation failed with errors.
    +>>create table t009t1 like "_HV_SCH_T009_".t009t1;
    +
    +*** ERROR[1010] The statement just entered is currently not supported.
    +
    +--- SQL operation failed with errors.
    +>>create table t009t2 as select * from "_HV_SCH_T009_".t009t2;
    +
    +*** ERROR[4258] Trying to access external table TRAFODION."_HV_SCH_T009_".T009T2 through its external name format. Please use the native table name.
    +
    +*** ERROR[8822] The statement was not prepared.
    +
    +>>
    +>>-- this create succeeds
    +>>create table t009t1 as select * from hive.sch_t009.t009t1;
    +
    +--- 10 row(s) inserted.
    +>>
    +>>get tables;
    +
    +Tables in Schema TRAFODION.HIVE_T009
    +====================================
    +
    +T009T1
    +
    +--- SQL operation complete.
    +>>drop table t009t1;
    +
    +--- SQL operation complete.
    +>>
     >>drop external table "_HV_HIVE_".customer;
     
     --- SQL operation complete.
     >>drop external table item for hive.hive.item;
     
     --- SQL operation complete.
    +>>
    +>>obey TEST009(test_hive2);
    +>>-- drop data from the hive table and recreate with 4 columns
    +>>-- this causes the external table to be invalid
    +>>
    +>>-- cleanup data from the old table, and create/load data with additional column
    +>>sh regrhadoop.ksh fs -rm   /user/hive/exttables/t009t1/*;
    +>>sh regrhive.ksh -v -f $REGRTSTDIR/TEST009_b.hive.sql;
    +>>
    +>>-- should fail - column mismatch
    +>>select count(*) from hive.sch_t009.t009t1;
    +
    +*** ERROR[3078] The column list for HIVE.SCH_T009.T009T1 does not match its external table representation defined as TRAFODION."_HV_SCH_T009_".T009T1.
    --- End diff --
    
    Should this be a warning instead of an error? The exernal table is not being directly used here. Should the user be stopped and forced to make sure that the two definitions are consistent?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38950251
  
    --- Diff: core/sql/common/ComMisc.cpp ---
    @@ -99,17 +100,74 @@ NABoolean ComIsTrafodionReservedSchemaName(
       return FALSE;
     }
     
    -// schema names of pattern "_HV ... _" and "_HB_ ... _" are reserved to store
    +// schema names of pattern "_HV_ ... _" and "_HB_ ... _" are reserved to store
     // external hive and hbase tables
     NABoolean ComIsTrafodionExternalSchemaName (
                                         const NAString &schName)
     {
       Int32 len (schName.length());
    +
    +  // check for HIVE
       Int32 prefixLen = sizeof(HIVE_EXT_SCHEMA_PREFIX);
       if (len > prefixLen && 
    -      (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX || 
    -       schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX) && 
    -      schName(len-1) == '_' )
    +     (schName(0,prefixLen-1) == HIVE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
         return TRUE;
    +
    +  // check for HBASE
    +  prefixLen = sizeof(HBASE_EXT_SCHEMA_PREFIX);
    +  if (len > prefixLen && 
    +     (schName(0,prefixLen-1) == HBASE_EXT_SCHEMA_PREFIX && 
    +      schName(len-1) == '_' ))
    +    return TRUE;
    +
       return FALSE;
     }
    +
    +// ----------------------------------------------------------------------------
    +// function: ComConvertNativeNameToTrafName
    +//
    +// this fuction converts the native HIVE or HBASE object name into its
    +// Trafodion external name format.
    +//
    +// params:
    +//    catalogName - catalog name to identify HBASE or HIVE native table
    +//    schemaName - external name of the HBASE or HIVE schema
    +//    objectName - external name of the HBASE of HIVE table
    +//
    +// If it is not HIVE or HBASE, just return the qualified name
    +// ----------------------------------------------------------------------------
    +NAString ComConvertNativeNameToTrafName ( 
    +  const NAString &catalogName,
    +  const NAString &schemaName,
    +  const NAString &objectName)
    +{
    +  // generate new schema name 
    +  NAString tempSchemaName; 
    +  if (catalogName == HIVE_SYSTEM_CATALOG)
    +    tempSchemaName += HIVE_EXT_SCHEMA_PREFIX;
    +  else if(catalogName == HBASE_SYSTEM_CATALOG)
    +    tempSchemaName += HBASE_EXT_SCHEMA_PREFIX;
    +  else
    +    return catalogName + NAString(".") +
    +           schemaName + NAString(".") +
    +           objectName; 
    +
    +  ComAnsiNamePart externalAnsiName(schemaName, ComAnsiNamePart::EXTERNAL_FORMAT);
    +  tempSchemaName += externalAnsiName.getInternalName();
    +  tempSchemaName.append ("_");
    +
    +  // Catalog name is "TRAFODION"
    +  NAString convertedName (CmpSeabaseDDL::getSystemCatalogStatic());
    +  convertedName += ".";
    +
    +  // append transformed schema name, convert internal name to external format
    +  ComAnsiNamePart internalAnsiName(tempSchemaName, ComAnsiNamePart::INTERNAL_FORMAT);
    +  convertedName += internalAnsiName.getExternalName();
    +
    +  // object  name is appended without change
    --- End diff --
    
    A question: Do we know that object names always conform to regular identifier syntax? Or if they don't, are they already in delimited form when we enter this method?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: Support for features described i...

Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/74#discussion_r38953383
  
    --- Diff: core/sql/parser/StmtDDLDrop.cpp ---
    @@ -561,7 +561,8 @@ StmtDDLDropSchema::StmtDDLDropSchema(//const SchemaName & schemaName,
       // If the schema name specified is reserved name, users cannot drop them.
       // They can only be dropped internally.
       if ((! Get_SqlParser_Flags(INTERNAL_QUERY_FROM_EXEUTIL)) &&
    -      (ComIsTrafodionReservedSchemaName(schemaQualName_.getSchemaName())))
    +      (ComIsTrafodionReservedSchemaName(schemaQualName_.getSchemaName())) &&
    +      (!ComIsTrafodionExternalSchemaName(schemaQualName_.getSchemaName())))
    --- End diff --
    
    Should this be a ! on line 565? I would have expected the condition to be similar to line 564. If I am wrong please excuse my mistake. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---