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 2016/03/08 19:01:53 UTC

[GitHub] incubator-trafodion pull request: TRAFODION [1879] - Integrate Lib...

GitHub user robertamarton opened a pull request:

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

    TRAFODION [1879] - Integrate Library Management into Trafodion Metadata

    The initial release of library management has been delivered to Trafodion. This
    is follow-on work that integrates library management operations into the
    existing Trafodion infrastructure. Currently, with the initial release of
    library management, the consumer needs to run a special script to set up
    everything. This delivery incorporates the steps into existing SQL commands and
    removes the need for the script.

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

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

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

    https://github.com/apache/incubator-trafodion/pull/372.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 #372
    
----
commit 9e3dea641f54995fd5adbdbf724dae9c10dbfe40
Author: Roberta Marton <ro...@apache.org>
Date:   2016-03-08T18:00:03Z

    TRAFODION [1879] - Integrate Library Management into Trafodion Metadata
    
    The initial release of library management has been delivered to Trafodion. This
    is follow-on work that integrates library management operations into the
    existing Trafodion infrastructure. Currently, with the initial release of
    library management, the consumer needs to run a special script to set up
    everything. This delivery incorporates the steps into existing SQL commands and
    removes the need for the script.
    
    This delivery contains:
    
    -> Support for three new INITIALIZE TRAFODION options
    -> Support for a new role (plus infrastructure to make it easier to add roles)
    -> Change initialize authorization to handle upgrade better
    -> Fixed couple issues found while testing
    -> New regression test (udr/TEST102)
    
    *** Support for three new INITIALIZE TRAFODION options:
    
    Three new INITIALIZE TRAFODION options have been added:
       CREATE LIBRARY MANAGEMENT - create and populates the libmgr schema
       DROP LIBRARY MANAGEMENT - drops the libmgr schema
       UPGRADE LIBRARY MANAGEMENT - adds new procedures to the libmgr library
    
    Parser was changed to support these new options, a new keyword - MANAGEMENT
    was added.
      (parser/ParKeyWords.cpp/sqlparser.y)
    
    DDL compiler was changed to recognize the new options and call implementation
    methods.
      (optimizer/RelExeUtil, sqlcomp/CmpSeabaseDDLcommon.cpp)
    
    The following implementation methods were added:
      createSeabaseLibmgr (code for create library management)
      dropSeabaseLibmgr (code for drop library management)
      upgradeSeabaseLibmgr(code for upgrade library management)
      createLibmgrProcs – a helper method called by create and upgrade code to
                          create libmgr procedures
      grantLibmgrPrivs – a helper method called by createLibmgrProcs and
                         initialize authorization that add grants to procedures for
                         the new DB__LIBMGRROLE role.
       (CmpSeabaseDDL.h/CmpSeabaseDDLroutine.cpp)
    
    Changes were made to define the new schema, library and procedures.
      (common/ComSmallDefs.h, CmpSeabaseDDLroutine.h)
    
    All procedures are defined in a new file called sqlcomp/CmpSeabaseDDLroutine.h.
    This file is based on similar support for system metadata tables
    (CmpSeabaseDDLmd.h).  It contains the text of all the procedures.  During
    "create library management" and "upgrade library management" operations, this
    list is used to create/add procedures.
    
    *** Support for a new role:
    
    Initialize authorization code was changed to create the new DB__LIBMGRROLE role.
    As part of this change, role infrastructure was modified to make it easier to
    add system roles in the future. The initialize code checks to see what roles
    have not been added, and adds them.
      (common/NAUserId.h, common/ComUser, sqlcomp/PrivMgrMD,
       sqlcomp/CmpSeabaseDDLauth, sqlcomp/PrivMgrRoles)
    
    In addition, a check is made – if the SEABASE_LIBMGR_SCHEMA exists, then
    grants are performed on the procedures to allow DB__LIBMGRROLE execute
    privilege.
      (sqlcomp/CmpSeabaseDDLcommon.cpp)
    
    *** Change initialize authorization to handle upgrade better:
    
    Changes were made so initialize authorization performs an upgrade operation if
    called and component privileges already exists.
      (sqlcomp/PrivMgrComponentOperations, sqlcomp/PrivMgrComponentPrivileges,
       sqlcomp/PrivMgrMD)
    
    *** Fixed couple of issues found while testing:
    
    Invalid role ID generation.  Role ID generation code was using the wrong range
    to determine the next role ID.
      (CmpSeabaseDDLauth.cpp)
    
    Drop schema issues with libraries.  Drop library cascade fails when the order
    of libraries and functions is incorrect.  If you drop function, drop library it
    fails with missing procedure.
      (CmpSeabaseDDLschema.cpp)

----


---
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: TRAFODION [1879] - Integrate Lib...

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/372#discussion_r55450475
  
    --- Diff: core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp ---
    @@ -1425,3 +1426,169 @@ short CmpSeabaseDDL::validateRoutine(ExeCliInterface *cliInterface,
       return -1;
     
     } // CmpSeabaseDDL::validateRoutine
    +
    +short CmpSeabaseDDL::createSeabaseLibmgr(ExeCliInterface * cliInterface)
    +{
    +  Lng32 cliRC = 0;
    +  
    +  if ((CmpCommon::context()->isUninitializedSeabase()) &&
    +      (CmpCommon::context()->uninitializedSeabaseErrNum() == -1393))
    +    {
    +      *CmpCommon::diags() << DgSqlCode(-1393);
    +      return -1;
    +    }
    +
    +  NAString jarLocation(getenv("MY_SQROOT"));
    +  jarLocation += "/export/lib/lib_mgmt.jar";
    +  char queryBuf[strlen(getSystemCatalog()) + strlen(SEABASE_LIBMGR_SCHEMA) +
    +                strlen(SEABASE_LIBMGR_LIBRARY) + strlen(DB__LIBMGRROLE) + 
    +                jarLocation.length() + 100];
    +
    +  // Create the SEABASE_LIBMGR_SCHEMA schema
    +  str_sprintf(queryBuf, "create schema if not exists %s.\"%s\" authorization %s ",
    +              getSystemCatalog(),SEABASE_LIBMGR_SCHEMA, DB__ROOT);
    +
    +  cliRC = cliInterface->executeImmediate(queryBuf);
    +  if (cliRC < 0)
    +    {
    +      cliInterface->retrieveSQLDiagnostics(CmpCommon::diags());
    +      return -1;
    +    }
    +
    +  // Create the SEABASE_LIBMGR_LIBRARY library
    +  str_sprintf(queryBuf, "create library %s.\"%s\".%s file '%s'",
    +                         getSystemCatalog(), SEABASE_LIBMGR_SCHEMA, SEABASE_LIBMGR_LIBRARY,
    +                         jarLocation.data());
    +
    +  cliRC = cliInterface->executeImmediate(queryBuf);
    +  if (cliRC < 0)
    +    {
    +      cliInterface->retrieveSQLDiagnostics(CmpCommon::diags());
    +      return -1;
    +    }
    +
    +  return (createLibmgrProcs(cliInterface));
    +}
    +
    +short CmpSeabaseDDL::createLibmgrProcs(ExeCliInterface * cliInterface)
    +{
    +  Lng32 cliRC = 0;
    +  char queryBuf[2000];
    +
    + // Create the procedures if they don't already exist
    +  for (Int32 i = 0; i < sizeof(allLibmgrRoutineInfo)/sizeof(LibmgrRoutineInfo); i++)
    +    {
    +      // Get the next procedure routine details
    +      const LibmgrRoutineInfo &prd = allLibmgrRoutineInfo[i];
    +
    +      const QString * qs = NULL;
    +      Int32 sizeOfqs = 0;
    +
    +      qs = prd.newDDL;
    +      sizeOfqs = prd.sizeOfnewDDL;
    +
    +      Int32 qryArraySize = sizeOfqs / sizeof(QString);
    +      char * gluedQuery;
    +      Lng32 gluedQuerySize;
    +      glueQueryFragments(qryArraySize,  qs,
    +                         gluedQuery, gluedQuerySize);
    +
    +      param_[0] = getSystemCatalog();
    +      param_[1] = SEABASE_LIBMGR_SCHEMA;
    +      param_[2] = getSystemCatalog();
    +      param_[3] = SEABASE_LIBMGR_SCHEMA;
    +      param_[4] = SEABASE_LIBMGR_LIBRARY;
    +
    +      str_sprintf(queryBuf, gluedQuery, param_[0], param_[1], param_[2], param_[3], param_[4]);
    +      NADELETEBASIC(gluedQuery, STMTHEAP);
    --- End diff --
    
    How do we know that queryBuf is large enough to hold the data str_sprintf writes to 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: TRAFODION [1879] - Integrate Lib...

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/372#discussion_r55619852
  
    --- Diff: core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp ---
    @@ -1425,3 +1426,169 @@ short CmpSeabaseDDL::validateRoutine(ExeCliInterface *cliInterface,
       return -1;
     
     } // CmpSeabaseDDL::validateRoutine
    +
    +short CmpSeabaseDDL::createSeabaseLibmgr(ExeCliInterface * cliInterface)
    +{
    +  Lng32 cliRC = 0;
    +  
    +  if ((CmpCommon::context()->isUninitializedSeabase()) &&
    +      (CmpCommon::context()->uninitializedSeabaseErrNum() == -1393))
    +    {
    +      *CmpCommon::diags() << DgSqlCode(-1393);
    +      return -1;
    +    }
    +
    +  NAString jarLocation(getenv("MY_SQROOT"));
    +  jarLocation += "/export/lib/lib_mgmt.jar";
    +  char queryBuf[strlen(getSystemCatalog()) + strlen(SEABASE_LIBMGR_SCHEMA) +
    +                strlen(SEABASE_LIBMGR_LIBRARY) + strlen(DB__LIBMGRROLE) + 
    +                jarLocation.length() + 100];
    +
    +  // Create the SEABASE_LIBMGR_SCHEMA schema
    +  str_sprintf(queryBuf, "create schema if not exists %s.\"%s\" authorization %s ",
    +              getSystemCatalog(),SEABASE_LIBMGR_SCHEMA, DB__ROOT);
    +
    +  cliRC = cliInterface->executeImmediate(queryBuf);
    +  if (cliRC < 0)
    +    {
    +      cliInterface->retrieveSQLDiagnostics(CmpCommon::diags());
    +      return -1;
    +    }
    +
    +  // Create the SEABASE_LIBMGR_LIBRARY library
    +  str_sprintf(queryBuf, "create library %s.\"%s\".%s file '%s'",
    +                         getSystemCatalog(), SEABASE_LIBMGR_SCHEMA, SEABASE_LIBMGR_LIBRARY,
    +                         jarLocation.data());
    +
    +  cliRC = cliInterface->executeImmediate(queryBuf);
    +  if (cliRC < 0)
    +    {
    +      cliInterface->retrieveSQLDiagnostics(CmpCommon::diags());
    +      return -1;
    +    }
    +
    +  return (createLibmgrProcs(cliInterface));
    +}
    +
    +short CmpSeabaseDDL::createLibmgrProcs(ExeCliInterface * cliInterface)
    +{
    +  Lng32 cliRC = 0;
    +  char queryBuf[2000];
    +
    + // Create the procedures if they don't already exist
    +  for (Int32 i = 0; i < sizeof(allLibmgrRoutineInfo)/sizeof(LibmgrRoutineInfo); i++)
    +    {
    +      // Get the next procedure routine details
    +      const LibmgrRoutineInfo &prd = allLibmgrRoutineInfo[i];
    +
    +      const QString * qs = NULL;
    +      Int32 sizeOfqs = 0;
    +
    +      qs = prd.newDDL;
    +      sizeOfqs = prd.sizeOfnewDDL;
    +
    +      Int32 qryArraySize = sizeOfqs / sizeof(QString);
    +      char * gluedQuery;
    +      Lng32 gluedQuerySize;
    +      glueQueryFragments(qryArraySize,  qs,
    +                         gluedQuery, gluedQuerySize);
    +
    +      param_[0] = getSystemCatalog();
    +      param_[1] = SEABASE_LIBMGR_SCHEMA;
    +      param_[2] = getSystemCatalog();
    +      param_[3] = SEABASE_LIBMGR_SCHEMA;
    +      param_[4] = SEABASE_LIBMGR_LIBRARY;
    +
    +      str_sprintf(queryBuf, gluedQuery, param_[0], param_[1], param_[2], param_[3], param_[4]);
    +      NADELETEBASIC(gluedQuery, STMTHEAP);
    --- End diff --
    
    2000 is big enough but I changed the approximate size calculation based on the size of the catalog, schema, library, and glued query - will deliver in my next release - coming soon.


---
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: TRAFODION [1879] - Integrate Lib...

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/372#discussion_r55449227
  
    --- Diff: core/sql/common/ComUser.cpp ---
    @@ -347,3 +347,57 @@ Int16 ComUser::getAuthNameFromAuthID(Int32   authID,
     
       return FEOK;
     }
    +
    +// ----------------------------------------------------------------------------
    +// method: getRoleList
    +//
    +// Returns the list of system roles
    +// Params:
    +//   (out) roleList - the list of roles, space is managed by the caller
    +//   (out) actualLen - the length of the returned role list
    +//   ( in) maxLen - the size of the roleList allocated by the caller
    +//   ( in) delimited - delimiter to use (defaults to single quote)
    +//   ( in) separator - specified what separator to use (defaults to comma)
    +//   ( in) includeSpecialAuths - includes the special auths (PUBLIC and _SYSTEM) 
    +//
    +//  Returns:  FEOK -- found
    +//            FEBUFTOOSMALL -- space allocated for role list is too small 
    +// ----------------------------------------------------------------------------
    +Int32 ComUser::getRoleList (char * roleList,
    --- End diff --
    
    I wonder if we could return an NAString instead, then we'd never have to trouble ourselves with FEBUFTOOSMALL? (At the price of dynamic heap management though.)


---
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: TRAFODION [1879] - Integrate Lib...

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/372#discussion_r55617875
  
    --- Diff: core/sql/common/ComUser.cpp ---
    @@ -347,3 +347,57 @@ Int16 ComUser::getAuthNameFromAuthID(Int32   authID,
     
       return FEOK;
     }
    +
    +// ----------------------------------------------------------------------------
    +// method: getRoleList
    +//
    +// Returns the list of system roles
    +// Params:
    +//   (out) roleList - the list of roles, space is managed by the caller
    +//   (out) actualLen - the length of the returned role list
    +//   ( in) maxLen - the size of the roleList allocated by the caller
    +//   ( in) delimited - delimiter to use (defaults to single quote)
    +//   ( in) separator - specified what separator to use (defaults to comma)
    +//   ( in) includeSpecialAuths - includes the special auths (PUBLIC and _SYSTEM) 
    +//
    +//  Returns:  FEOK -- found
    +//            FEBUFTOOSMALL -- space allocated for role list is too small 
    +// ----------------------------------------------------------------------------
    +Int32 ComUser::getRoleList (char * roleList,
    --- End diff --
    
    I thought about it, I tried to keep the code similar to other functions in the same file.  But you have a point. Will think about it for a subsequent release.


---
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: TRAFODION [1879] - Integrate Lib...

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

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


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