You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by anoopsharma00 <gi...@git.apache.org> on 2016/02/29 06:42:08 UTC
[GitHub] incubator-trafodion pull request: JIRA TRAFODION-1798 (ddl xns) an...
GitHub user anoopsharma00 opened a pull request:
https://github.com/apache/incubator-trafodion/pull/346
JIRA TRAFODION-1798 (ddl xns) and few other fixes, details below.
-- support for sql part of ddl xns. Section 1 of JIRA TRAFODION-1798
-- cqd ddl_transactions to enable or disable ddl xns.
Default is currently off. Once it is tested, it will be turned on.
Dev regressions are run with cqd set to ON
-- get stmts run with read committed to get changes in current xns
-- support for where preds with get stmts
-- scan to pass in transid even if running with read uncommitted access.
This enables rows modified in current xn to be returned.
-- cleanup no longer return multiple duplicate error messages if
objects id is not found.
-- cleanup no longer includes internallay created schemas (_HV_ , _HB_)
during cleanup operations.
-- Correct error msg was not getting returned if an invalid index
existed in table and the same index was created again.
-- init traf, drop md views was giving an error if views didnt exist.
That has been fixed.
-- regressions with -diff option now show original file timestamps
instead of the timestamp when the diff command was run.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anoopsharma00/incubator-trafodion deliver2_dt_br
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/346.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 #346
----
commit cd9f7a1ae65ac4aca485fb30b98daf8a1ccce885
Author: Cloud User <an...@apache.org>
Date: 2016-02-29T05:39:02Z
JIRA TRAFODION-1798 (ddl xns) and few other fixes, details below.
-- support for sql part of ddl xns. Section 1 of JIRA TRAFODION-1798
-- cqd ddl_transactions to enable or disable ddl xns.
Default is currently off. Once it is tested, it will be turned on.
Dev regressions are run with cqd set to ON
-- get stmts run with read committed to get changes in current xns
-- support for where preds with get stmts
-- scan to pass in transid even if running with read uncommitted access.
This enables rows modified in current xn to be returned.
-- cleanup no longer return multiple duplicate error messages if
objects id is not found.
-- cleanup no longer includes internallay created schemas (_HV_ , _HB_)
during cleanup operations.
-- Correct error msg was not getting returned if an invalid index
existed in table and the same index was created again.
-- init traf, drop md views was giving an error if views didnt exist.
That has been fixed.
-- regressions with -diff option now show original file timestamps
instead of the timestamp when the diff command was run.
----
---
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: JIRA TRAFODION-1798 (ddl xns) an...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/346
---
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: JIRA TRAFODION-1798 (ddl xns) an...
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/346#discussion_r54661059
--- Diff: core/sql/optimizer/NARoutineDB.h ---
@@ -50,7 +50,7 @@ class NARoutineDB : public NAKeyLookup<NARoutineDBKey,NARoutine>
public:
// This same enum exists in NATableDB, should probably only have one copy
- enum QiScope { REMOVE_FROM_ALL_USERS = 100, REMOVE_MINE_ONLY };
+ // enum QiScope { REMOVE_FROM_ALL_USERS = 100, REMOVE_MINE_ONLY };
--- End diff --
Could get rid of these comments?
---
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: JIRA TRAFODION-1798 (ddl xns) an...
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/346#discussion_r54661719
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLcleanup.cpp ---
@@ -1089,7 +1092,7 @@ short CmpSeabaseMDcleanup::cleanupOrphanObjectsEntries(ExeCliInterface *cliInter
NABoolean errorSeen = FALSE;
// find out all entries which do not have corresponsing hbase objects
- str_sprintf(query, "select object_uid, trim(catalog_name) || '.' || trim(schema_name) || '.' || trim(object_name) from %s.\"%s\".%s where catalog_name = '%s' and schema_name not in ( '_MD_', '_REPOS_', 'PRIVMGR_MD') and (object_type = 'BT' or object_type = 'IX') ",
+ str_sprintf(query, "select object_uid, trim(catalog_name) || '.' || trim(schema_name) || '.' || trim(object_name) from %s.\"%s\".%s where catalog_name = '%s' and schema_name not in ( '_MD_', '_REPOS_', '_PRIVMGR_MD_') and schema_name not like '|_HV|_%%' escape '|' and schema_name not like '|_HB|_%%' escape '|' and (object_type = 'BT' or object_type = 'IX') ",
--- End diff --
Are schema names beginning with _HV and _HB reserved? If not, then this statement might mistakenly filter out user schemas (like "_HVAC").
---
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: JIRA TRAFODION-1798 (ddl xns) an...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/346#discussion_r54648838
--- Diff: core/sqf/src/seatrans/hbase-trx/src/main/java/org/apache/hadoop/hbase/client/transactional/TransactionalScanner.java ---
@@ -171,19 +171,21 @@ protected boolean nextScanner(final boolean done) throws IOException{
return false;
}
else
- this.currentBeginKey = TransactionManager.binaryIncrementPos(this.currentEndKey,1);
+ //this.currentBeginKey = TransactionManager.binaryIncrementPos(this.currentEndKey,1);
+ this.currentBeginKey = this.currentEndKey;
}
else {
// First call to nextScanner
this.currentBeginKey = this.scan.getStartRow();
}
- this.currentRegion = ttable.getRegionLocation(this.currentBeginKey).getRegionInfo();
+ this.currentRegion = ttable.getRegionLocation(this.currentBeginKey, false).getRegionInfo();
this.currentEndKey = this.currentRegion.getEndKey();
- if(LOG.isTraceEnabled()) LOG.trace("Region Info: " + currentRegion.getRegionNameAsString());
- if(this.currentEndKey != HConstants.EMPTY_END_ROW)
- this.currentEndKey = TransactionManager.binaryIncrementPos(currentRegion.getEndKey(), -1);
+ if(LOG.isTraceEnabled()) LOG.trace("nextScanner() txID: " + ts.getTransactionId() + " Region Info: " + currentRegion.getRegionNameAsString()+
+ "currentBeginKey: " + Hex.encodeHexString(this.currentBeginKey) + "currentEndKey: " + Hex.encodeHexString(this.currentEndKey));
+ //if(this.currentEndKey != HConstants.EMPTY_END_ROW)
--- End diff --
Consider removing the commented out lines
---
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: JIRA TRAFODION-1798 (ddl xns) an...
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/346#discussion_r54661369
--- Diff: core/sql/parser/StmtDDLAlter.cpp ---
@@ -1971,6 +1971,17 @@ StmtDDLAlterTableAddColumn::synthesize()
theColumn->traverseList(
this,
StmtDDLAlterTableAddColumn_visit);
+
+ ElemDDLColDefArray ColDefArray = getColDefArray();
+ ElemDDLColDef *pColDef = ColDefArray[0];
+ if (NOT pColDef->getColumnFamily().isNull())
+ {
+ //TEMPTEMP
+ // Currently, DTM doesnt handle add columns with an explicit
+ // column family as a transactional operation.
+ // Do not use ddl xns until that bug is fixed.
+ setDdlXns(FALSE);
+ }
--- End diff --
How about ALTER TABLE/INDEX ALTER HBASE_OPTIONS?
---
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: JIRA TRAFODION-1798 (ddl xns) an...
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/346#discussion_r54662095
--- Diff: core/sql/sqlcomp/CmpSeabaseDDLcommon.cpp ---
@@ -8167,35 +8239,71 @@ short CmpSeabaseDDL::executeSeabaseDDL(DDLExpr * ddlExpr, ExprNode * ddlNode,
}
NABoolean startXn = TRUE;
- if ((ddlExpr->dropHbase()) ||
- (ddlExpr->purgedataHbase()) ||
- (ddlExpr->initHbase()) ||
- (ddlExpr->createMDViews()) ||
- (ddlExpr->dropMDViews()) ||
- (ddlExpr->initAuthorization()) ||
- (ddlExpr->dropAuthorization()) ||
- (ddlExpr->addSeqTable()) ||
- (ddlExpr->createRepos()) ||
- (ddlExpr->dropRepos()) ||
- (ddlExpr->upgradeRepos()) ||
- (ddlExpr->addSchemaObjects()) ||
- (ddlExpr->updateVersion()) ||
- ((ddlNode) &&
- // TODO: When making ALTER TABLE/INDEX transactional, add cases here for them
- ((ddlNode->getOperatorType() == DDL_DROP_SCHEMA) ||
- (ddlNode->getOperatorType() == DDL_CLEANUP_OBJECTS) ||
- (ddlNode->getOperatorType() == DDL_ALTER_TABLE_ADD_CONSTRAINT_PRIMARY_KEY) ||
- (ddlNode->getOperatorType() == DDL_ALTER_TABLE_ALTER_COLUMN_SET_SG_OPTION) ||
- (ddlNode->getOperatorType() == DDL_CREATE_INDEX) ||
- (ddlNode->getOperatorType() == DDL_POPULATE_INDEX) ||
- (ddlNode->getOperatorType() == DDL_CREATE_TABLE) ||
- (ddlNode->getOperatorType() == DDL_ALTER_TABLE_DROP_COLUMN) ||
- (ddlNode->getOperatorType() == DDL_DROP_TABLE))))
+
+ // no DDL transactions.
+ if ((NOT ddlExpr->ddlXns()) &&
+ ((ddlExpr->dropHbase()) ||
+ (ddlExpr->purgedataHbase()) ||
+ (ddlExpr->initHbase()) ||
+ (ddlExpr->createMDViews()) ||
+ (ddlExpr->dropMDViews()) ||
+ (ddlExpr->initAuthorization()) ||
+ (ddlExpr->dropAuthorization()) ||
+ (ddlExpr->addSeqTable()) ||
+ (ddlExpr->createRepos()) ||
+ (ddlExpr->dropRepos()) ||
+ (ddlExpr->upgradeRepos()) ||
+ (ddlExpr->addSchemaObjects()) ||
+ (ddlExpr->updateVersion())))
+ {
+ // transaction will be started and commited in called methods.
+ startXn = FALSE;
+ }
+
+ // no DDL transactions
+ if (((ddlNode) && (ddlNode->castToStmtDDLNode()) &&
+ (NOT ddlNode->castToStmtDDLNode()->ddlXns())) &&
+ ((ddlNode->getOperatorType() == DDL_DROP_SCHEMA) ||
+ (ddlNode->getOperatorType() == DDL_CLEANUP_OBJECTS) ||
+ (ddlNode->getOperatorType() == DDL_ALTER_TABLE_ADD_CONSTRAINT_PRIMARY_KEY) ||
+ (ddlNode->getOperatorType() == DDL_ALTER_TABLE_ALTER_COLUMN_SET_SG_OPTION) ||
+ (ddlNode->getOperatorType() == DDL_CREATE_INDEX) ||
+ (ddlNode->getOperatorType() == DDL_POPULATE_INDEX) ||
+ (ddlNode->getOperatorType() == DDL_CREATE_TABLE) ||
+ (ddlNode->getOperatorType() == DDL_ALTER_TABLE_DROP_COLUMN) ||
+ (ddlNode->getOperatorType() == DDL_DROP_TABLE)))
+ {
--- End diff --
Earlier I had asked about ALTER TABLE/INDEX ALTER HBASE_OPTIONS... maybe that one should be here 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: JIRA TRAFODION-1798 (ddl xns) an...
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/346#discussion_r54659917
--- Diff: core/sql/cli/Context.h ---
@@ -472,6 +477,10 @@ class ContextCli : public ExGod {
bool cbServerInUse_;
NABoolean dropInProgress_;
+ // set to true if ddl stmts are issued.
+ // Reset at begin and commit Xn. Used to do NATable invalidation
+ // for ddl stmts issued within a transaction.
--- End diff --
Glad you are doing this. This has often been a sore point.
---
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: JIRA TRAFODION-1798 (ddl xns) an...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/346#discussion_r54652316
--- Diff: core/sql/executor/ex_transaction.cpp ---
@@ -1316,11 +1309,29 @@ short ExTransTcb::work()
// create a diagsArea and return it to the parent.
handleErrors(pentry_down, ta->getDiagsArea());
+ if (cliGlobals->currContext()->ddlStmtsExecuted())
+ {
+ ComDiagsArea * diagsArea = NULL;
+ ExSqlComp::ReturnStatus cmpStatus =
+ cliGlobals->currContext()->sendXnMsgToArkcmp
+ (NULL, 0,
+ EXSQLCOMP::DDL_NATABLE_INVALIDATE,
+ diagsArea);
+ if (cmpStatus == ExSqlComp::ERROR)
+ {
+ handleErrors(pentry_down, NULL,
+ (ExeErrorCode)(-EXE_CANT_COMMIT_OR_ROLLBACK));
+
+ return -1;
+ }
+ }
+
+ cliGlobals->currContext()->ddlStmtsExecuted() = FALSE;
--- End diff --
The ddlStmtsExecuted flag will be set to TRUE if there is any error on commit or rollback. How can the user process recover from this,
---
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: JIRA TRAFODION-1798 (ddl xns) an...
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/346#discussion_r54660334
--- Diff: core/sql/executor/ExExeUtilGet.cpp ---
@@ -3173,6 +3149,21 @@ short ExExeUtilGetHbaseObjectsTcb::work()
}
break;
+ case EVAL_EXPR_:
--- End diff --
This is a very nice feature. Thanks for adding this. I've always wondered though why GET statements aren't simply SELECTs. Would be nice to embed them within other SELECTs for example. There have been times when I've wanted to join them to other things 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.
---