You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by traflm <gi...@git.apache.org> on 2018/08/10 13:33:04 UTC
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
GitHub user traflm opened a pull request:
https://github.com/apache/trafodion/pull/1688
[TRAFODION-3034] Support Oracle Hierarchy Query (Connect By)
This PR is the first patch to support Oracle Hierarchy query feature (CONNECT BY).
In this PR, the feature is implemented as a new SQL utility, It is standalone, rather clear isolated with all other SQL functions, so the impact is minimal.
In the long run, we should finish the ANSI recursive feature (recursive WITH) , and at that time, this feature can be considered to refactor to use that infrastructure.
This is just the first phase of this feature.
The basic logic is simple: the utility will run a query to get all start values (specified by the START WITH clause), then it will construct queries to search for children of the root, and loop until no children can be found.
Oracle has 3 pseudo columns, to support the ISLEAF and CONNECT_BY_PATH, the utility will have to run a query for each parent, it will be rather slow. If a query doesn't have those two pseudo columns required, the utility will run in batch mode, for each iteration, get all children in one query. That will be much faster.
One can check the executor/TEST021 for how this feature works first.
This will be a long review process, there must be many places to be modified and enhanced, thank you all for help in advance.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/traflm/trafodion TRAFODION-3034
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1688.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 #1688
----
commit 5fb8f21f20c397e077b2f7a8f96a7be9819f4883
Author: Liu Ming <ov...@...>
Date: 2018-06-01T22:44:58Z
[TRAFODION-3034] add parser changes
commit b29f4ddb417b087a3c888b17e9fae2ee6b060828
Author: Liu Ming <ov...@...>
Date: 2018-07-17T09:21:17Z
basic work finish
commit 88e8b20d91ed7093ea06448a64bcdc3090d91ba9
Author: Liu Ming <ov...@...>
Date: 2018-07-21T09:56:44Z
fix parser, add loop detection
commit 9fb3a5943f6440540b4894bc554d94b317fd9ece
Author: Liu Ming <ov...@...>
Date: 2018-07-23T05:28:08Z
refactor the code, next need to add CQD and where clause support
commit b875ec3cdaef2c33cc3d127c24e62be478a78781
Author: Liu Ming <ov...@...>
Date: 2018-07-25T04:22:21Z
support where clause, next add CQD to control memory usage
commit 1b853c5da8210d77bd8d84dac374e57a67e38bef
Author: Liu Ming <ov...@...>
Date: 2018-07-26T05:00:23Z
support where clause, next add regression test
commit f20b63c875f3a56ba3da1a83c2f1e0f255b29b71
Author: Liu Ming <ov...@...>
Date: 2018-07-27T12:03:44Z
support both where clause and order by, next add test cases
commit 5757f25e9c2fa91b25751e24f28b04fbf903df4e
Author: Liu Ming <ov...@...>
Date: 2018-07-28T02:54:56Z
add test case, next to rebase and try to support is_cycle
commit 73a175a19c4f77e2af761ecc2a0b01779f9b7175
Author: Liu Ming <ov...@...>
Date: 2018-07-28T05:15:14Z
Merge branch 'master' of git://git.apache.org/trafodion into TRAFODION-3034
Conflicts:
core/sql/generator/GenRelExeUtil.cpp
core/sql/optimizer/RelExeUtil.h
core/sql/sqlcomp/DefaultConstants.h
commit 1b32b6b27b7ba4686b174c41a349f320807221fe
Author: Liu Ming <ov...@...>
Date: 2018-08-04T10:44:33Z
support PATH, next try to support is leaf
commit 81ef30d0e97797e033522b711f1aeb5ba359cd94
Author: Liu Ming <ov...@...>
Date: 2018-08-05T09:43:03Z
add ISLEAF support, but need to wait for another enhancement
commit d781d881ad400e75fcd765d1856bdb509e2c6adb
Author: Liu Ming <ov...@...>
Date: 2018-08-07T20:48:56Z
finish is leaf
commit cc9057d27aec9d0e5413330f72cc8df932103e7d
Author: Liu Ming <ov...@...>
Date: 2018-08-07T20:49:08Z
Merge branch 'master' of git://git.apache.org/trafodion into TRAFODION-3034
commit 8ae2ed9c5fcbb1ccb47f6b952438dedc06b0d90e
Author: Liu Ming <ov...@...>
Date: 2018-08-10T05:27:13Z
first phase finished
----
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221489667
--- Diff: core/sql/comexe/ComTdbExeUtil.cpp ---
@@ -3028,3 +3028,59 @@ Lng32 ComTdbExeUtilLobInfo::unpack(void * base, void * reallocator)
return -1;
return ComTdbExeUtil::unpack(base, reallocator);
}
+
+ComTdbExeUtilConnectby::ComTdbExeUtilConnectby(char * query,
+ ULng32 querylen,
+ Int16 querycharset,
+ char * tableName,
+ char * stmtName,
+ ex_expr * input_expr,
+ ULng32 input_rowlen,
+ ex_expr * output_expr,
+ ULng32 output_rowlen,
+ ex_expr * scan_expr,
+ ex_cri_desc * work_cri_desc,
+ const unsigned short work_atp_index,
+ Lng32 colDescSize,
+ Lng32 outputRowSize,
+ ex_cri_desc * given_cri_desc,
+ ex_cri_desc * returned_cri_desc,
+ queue_index down,
+ queue_index up,
+ Lng32 num_buffers,
+ ULng32 buffer_size,
+ ExCriDescPtr workCriDesc
+ )
+ : ComTdbExeUtil(ComTdbExeUtil::CONNECT_BY_,
+ query, querylen, querycharset,
+ tableName, strlen(tableName),
+ input_expr, input_rowlen,
+ output_expr, output_rowlen,
+ scan_expr,
+ work_cri_desc, work_atp_index,
+ given_cri_desc, returned_cri_desc,
+ down, up,
+ num_buffers, buffer_size),
+ flags_(0),
+ myWorkCriDesc_(workCriDesc),
+ tupleLen_(outputRowSize)
+{
+ setNodeType(ComTdb::ex_CONNECT_BY);
+ connTableName_ = tableName;
+ maxDeep_ = 200; //by default, max deep of a tree
--- End diff --
These two literals could be consts to improve readability. It looks like there are cqds defined to make these configurable, which is excellent.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221496330
--- Diff: core/sql/parser/ParKeyWords.cpp ---
@@ -862,7 +863,7 @@ ParKeyWord ParKeyWords::keyWords_[] = {
ParKeyWord("PREPARE", TOK_PREPARE, ANS_|RESWORD_),
ParKeyWord("PRESERVE", TOK_PRESERVE, ANS_|RESWORD_),
ParKeyWord("PRIMARY", TOK_PRIMARY, FIRST_|ANS_|RESWORD_),
- ParKeyWord("PRIOR", IDENTIFIER, ANS_|RESWORD_),
+ ParKeyWord("PRIOR", PRIOR_IDENTIFIER, ANS_),
--- End diff --
Could you please explain why this is a PRIOR_IDENTIFIER and not TOK_PRIOR? Are we attempting to allow use of PRIOR as an identifier now. It was previously not allowed, since it was a RESWORD. Are we trying to have fewer reserved words? This would be an admirable goal. Thank you for explaining, this is not a defect.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221819747
--- Diff: core/sql/optimizer/RelExpr.h ---
@@ -1439,6 +1459,7 @@ class RelExpr : public ExprNode
enum Flags {
EXPAND_SHORT_ROWS = 0x00000001 // expand short rows when added columns
,PARENT_IS_ROOT = 0x00000002 // compressed internal format
+ ,HAS_CONNECT_BY = 0x00000004 // compressed internal format
--- End diff --
) typo on the comment.
2) why do we need a bit flag on the RelExpr? Can we just check nullness of biConnectBy and other data members of this class? I was under the impression that we did not need bit flags in RelExpr data members in the compiler till be reached the generator. This could be an incorrect idea.
3) Can any RelExpr have a CONNECT_BY. I am assuming there are several RelExpr where this is not allowed/supported. We should add binder error for those that are not caught in the parser, if any.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221489512
--- Diff: core/sql/bin/SqlciErrors.txt ---
@@ -1515,6 +1515,9 @@ $1~String1 --------------------------------
8034 ZZZZZ 99999 ADVANCED MAJOR DBADMIN Column $0~String0 of object $1~string1 does not have a default clause but it is missing in database. This indicates inconsistent data.
8035 ZZZZZ 99999 ADVANCED MAJOR DBADMIN Truncation of hive table failed. $0~String0
8036 ZZZZZ 99999 ADVANCED MINOR LOGONLY Error while creating the error logging file or logging the error row to file $0~String0: Details :$1~String1
+8037 ZZZZZ 99999 BEGINNER MAJOR DBADMIN Loop detected in connect by execution.
--- End diff --
In these three messages, it would help if we could give information specific to the instance when the error occurred. Maybe value of the prior column(s) as the error occurred. For the case of recursion exceeding a certain length or memory being exhausted, more details what level of recursion has been reached or how much memory has been consumed (if that information is readily available) would be helpful. This is an advisory suggestion, and by no means something to be addressed soon.
I would have expected some binder errors for statements where hierarchical constructs are not supported. It is possible though unlikely that all are caught in the parser. For example what happens if a hierarchical function or pseudocolumn is used a regular (non-hierarchical query)
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221820929
--- Diff: core/sql/optimizer/ValueDesc.cpp ---
@@ -4555,7 +4555,7 @@ void ValueIdSet::unparse(NAString &result,
NAString connectorText;
- if ((form == MVINFO_FORMAT) || (form == QUERY_FORMAT))
+ if ((form == MVINFO_FORMAT) || (form == QUERY_FORMAT) || (form == CONNECT_BY_FORMAT))
--- End diff --
How is the CONNECT_BY_FORMAT different from QUERY or MVINFO formats? The answer might be in other parts of code, its just that I cannot tell. Thank you for explaining
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221818361
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13280,13 +13288,130 @@ table_expression : from_clause where_clause sample_clause
SqlParser_CurrentParser->topHasOlapFunctions());
SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
}
+ | from_clause startwith_clause where_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
+ {
+ $$ =
+ getTableExpressionRelExpr($1,
+ $3,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ }
+ else
+ $$ =
+ getTableExpressionRelExpr($1,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
+ ((BiConnectBy*)$2)->where_clause = $3;
+ $$->setBiConnectBy( $2);
+ $$->setHasConnectByFlag(TRUE);
+ }
+ | from_clause TOK_WHERE search_condition startwith_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
+ $$ =
+ getTableExpressionRelExpr($1,
+ $3,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ else
+ $$ =
+ getTableExpressionRelExpr($1,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
+ ((BiConnectBy*)$4)->where_clause = $3;
+ //((BiConnectBy*)$3)->order_siblings_by_clause = $4;
+ $$->setBiConnectBy( $4);
+ $$->setHasConnectByFlag(TRUE);
+ }
/* type relx */
from_clause : TOK_FROM global_hint table_reference { $$ = $3; }
| from_clause ',' table_reference
{
$$ = new (PARSERHEAP()) Join($1, $3, REL_JOIN);
- }
+ }
+startwith_clause :TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY search_condition
+ {
+ $$ = new (PARSERHEAP())BiConnectBy ((BiRelat*)$2, (BiRelat*)$5);
+ //save the predicate text
+ $2->unparse(((BiConnectBy*)$$)->startWithString_, PARSER_PHASE, USER_FORMAT);
+ }
+ |TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY TOK_NOCYCLE search_condition
+ {
+ $$ = new (PARSERHEAP())BiConnectBy ((BiRelat*)$2, (BiRelat*)$6);
+ //save the predicate text
+ $2->unparse(((BiConnectBy*)$$)->startWithString_, PARSER_PHASE, USER_FORMAT);
+ ((BiConnectBy*)$$)->setNoCycle(TRUE);
+ }
+/*
+ | TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY search_condition order_by_clause
+ {
+ $$ = new (PARSERHEAP())BiConnectBy ((BiRelat*)$2, (BiRelat*)$5);
+ //save the predicate text
+ $2->unparse(((BiConnectBy*)$$)->startWithString_, PARSER_PHASE, USER_FORMAT);
+ ((BiConnectBy*)$$)->order_siblings_by_clause = $5;
+ }
+ |TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY TOK_NOCYCLE search_condition order_by_clause
+ {
+ $$ = new (PARSERHEAP())BiConnectBy ((BiRelat*)$2, (BiRelat*)$6);
+ //save the predicate text
+ $2->unparse(((BiConnectBy*)$$)->startWithString_, PARSER_PHASE, USER_FORMAT);
+ ((BiConnectBy*)$$)->setNoCycle(TRUE);
+ ((BiConnectBy*)$$)->order_siblings_by_clause = $6;
+ }
+*/
+ | CONNECT_IDENTIFIER TOK_BY search_condition
+ {
+ $$ = new (PARSERHEAP())BiConnectBy (NULL, (BiRelat*)$3);
+ }
+ | CONNECT_IDENTIFIER TOK_BY TOK_NOCYCLE search_condition
+ {
+ $$ = new (PARSERHEAP())BiConnectBy (NULL, (BiRelat*)$4);
+ ((BiConnectBy*)$$)->setNoCycle(TRUE);
+ }
+/*
+ | CONNECT_IDENTIFIER TOK_BY search_condition order_by_clause
+ {
+ $$ = new (PARSERHEAP())BiConnectBy (NULL, (BiRelat*)$3);
+ ((BiConnectBy*)$$)->order_siblings_by_clause = $4;
+ }
+ | CONNECT_IDENTIFIER TOK_BY TOK_NOCYCLE search_condition order_by_clause
+ {
+ $$ = new (PARSERHEAP())BiConnectBy (NULL, (BiRelat*)$4);
+ ((BiConnectBy*)$$)->setNoCycle(TRUE);
+ ((BiConnectBy*)$$)->order_siblings_by_clause = $5;
--- End diff --
I suppose this is for later as it is commented out now. Will there be a check for existence of SIBLINGS keyword in the binder. Maybe this is a good place to check it as previous rule would have set the sibling flag in $5?
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221819571
--- Diff: core/sql/optimizer/SynthType.cpp ---
@@ -7291,3 +7291,11 @@ const NAType * SplitPart::synthesizeType()
);
}
+
+const NAType * ItmSysConnectByPathFunc::synthesizeType()
+{
+ NAType * type;
+ type = new HEAP
+ SQLVarChar(HEAP, 3000, FALSE);
--- End diff --
The value 3000 seems arbitrary. It is not related to the column used in CONNECT_BY_PATH, nor to the depth of recursion. Consider making this settable by cqd. If that is not necessary, maybe a constant can be used to improve readability.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221817558
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13280,13 +13288,130 @@ table_expression : from_clause where_clause sample_clause
SqlParser_CurrentParser->topHasOlapFunctions());
SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
}
+ | from_clause startwith_clause where_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
+ {
+ $$ =
+ getTableExpressionRelExpr($1,
+ $3,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ }
+ else
+ $$ =
+ getTableExpressionRelExpr($1,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
+ ((BiConnectBy*)$2)->where_clause = $3;
+ $$->setBiConnectBy( $2);
+ $$->setHasConnectByFlag(TRUE);
+ }
+ | from_clause TOK_WHERE search_condition startwith_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
+ $$ =
+ getTableExpressionRelExpr($1,
+ $3,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ else
+ $$ =
+ getTableExpressionRelExpr($1,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
+ ((BiConnectBy*)$4)->where_clause = $3;
+ //((BiConnectBy*)$3)->order_siblings_by_clause = $4;
+ $$->setBiConnectBy( $4);
+ $$->setHasConnectByFlag(TRUE);
+ }
/* type relx */
from_clause : TOK_FROM global_hint table_reference { $$ = $3; }
| from_clause ',' table_reference
{
$$ = new (PARSERHEAP()) Join($1, $3, REL_JOIN);
- }
+ }
+startwith_clause :TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY search_condition
--- End diff --
From Oracle doc "In a hierarchical query, one expression in condition must be qualified with the PRIOR operator to refer to the parent row". Is this check done later?
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221824983
--- Diff: core/sql/executor/ExExeUtil.h ---
@@ -4172,6 +4173,48 @@ class ExExeUtilLobInfoPrivateState : public ex_tcb_private_state
protected:
};
+class connectByStackItem
--- End diff --
I think it is unusual in the executor to have classes that do not have a base class. How about memory management for this class. I might understand more as I see how these classes are used.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221821123
--- Diff: core/sql/optimizer/RelExpr.h ---
@@ -525,7 +540,7 @@ class RelExpr : public ExprNode
// QSTUFF
// --------------------------------------------------------------------
- // normalizeNode() performs predicate pushdown and also ensures
+ // normalizeNode() performs predicate pushdown and also ensuresL
--- End diff --
typo
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221816282
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13280,13 +13288,130 @@ table_expression : from_clause where_clause sample_clause
SqlParser_CurrentParser->topHasOlapFunctions());
SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
}
+ | from_clause startwith_clause where_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
--- End diff --
I am confused on several aspects here. None maybe issues
1) In line 13297 the WHERE clause is passed but not in line 13310. IF branch is taken if we have a Join. Why would the WHERE clause not be passed in for a single scan? Is it because it is already addressed in the preceding rule. If yes, what benefit do lines 13308 to 13318 provide?
2) Are OLAP functions in the select list supported? Should sequence functions be supported too (since they are similar)?
3) I wonder if this whole block of code could be expressed more concisely. There seems to be some redundancy now.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r209260745
--- Diff: core/sql/optimizer/BindRelExpr.cpp ---
@@ -2505,7 +2505,62 @@ RelExpr *RelExpr::bindSelf(BindWA *bindWA)
if (bindWA->inViewWithCheckOption())
bindWA->predsOfViewWithCheckOption() += selectionPred();
}
+#if 0
+ ItemExpr *startWithTree = removeStartWithTree();
+ if (startWithTree) {
+ bindWA->getCurrentScope()->context()->inWhereClause() = TRUE;
+ startWithTree->convertToValueIdSet(getStartWith(), bindWA, ITM_AND);
+ bindWA->getCurrentScope()->context()->inWhereClause() = FALSE;
+
+ if (bindWA->errStatus()) return this;
+ // If this is an embedded insert, then subquery predicates are not
+ // allowed.
+ // For example: To handle this query and issue an error stating
+ // subqueries are not allowed in embedded inserts
+ //
+ // select a from (insert into t901t01 values(22,22,222))t(a,b,c)
+ // where t.a IN (select m from t901t03 where t901t03.m = 77);
+
+ if (getGroupAttr()->isEmbeddedInsert())
+ {
+ if (!getStartWith().isEmpty() && getStartWith().containsSubquery())
+ {
+ *CmpCommon::diags() << DgSqlCode(-4337);
+ bindWA->setErrStatus();
+ return this;
+ }
+ }
+ }
+
+ ItemExpr *connectByTree = removeConnectByTree();
+ if (connectByTree) {
+
+ bindWA->getCurrentScope()->context()->inWhereClause() = TRUE;
+ connectByTree->convertToValueIdSet(getConnectBy(), bindWA, ITM_AND);
+ bindWA->getCurrentScope()->context()->inWhereClause() = FALSE;
+
+ if (bindWA->errStatus()) return this;
+
+ // If this is an embedded insert, then subquery predicates are not
+ // allowed.
+ // For example: To handle this query and issue an error stating
+ // subqueries are not allowed in embedded inserts
+ //
+ // select a from (insert into t901t01 values(22,22,222))t(a,b,c)
+ // where t.a IN (select m from t901t03 where t901t03.m = 77);
+
+ if (getGroupAttr()->isEmbeddedInsert())
+ {
+ if (!getConnectBy().isEmpty() && getConnectBy().containsSubquery())
+ {
+ *CmpCommon::diags() << DgSqlCode(-4337);
+ bindWA->setErrStatus();
+ return this;
+ }
+ }
+ }
+#endif
--- End diff --
oops, I will remove these dead code.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r223166271
--- Diff: core/sql/optimizer/ValueDesc.cpp ---
@@ -4555,7 +4555,7 @@ void ValueIdSet::unparse(NAString &result,
NAString connectorText;
- if ((form == MVINFO_FORMAT) || (form == QUERY_FORMAT))
+ if ((form == MVINFO_FORMAT) || (form == QUERY_FORMAT) || (form == CONNECT_BY_FORMAT))
--- End diff --
CONNECT_BY_FORMAT has only one difference from QUERY_FORMAT, that the column name only has the last part, not qualified.
The reason is: the column name will be used in the work() method of CONNECT_BY, where alias of the table name is lost. But we can make sure, there is only one table in that query in the work() method, so only the column name is required.
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221821938
--- Diff: core/sql/executor/ExExeUtil.h ---
@@ -4138,6 +4138,7 @@ class ExExeUtilLobInfoTableTcb : public ExExeUtilTcb
DONE_
};
Step step_;
+ char * data_;
--- End diff --
Why do we need a change in a Lob class?
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221818448
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13836,9 +13960,16 @@ query_spec_body : query_select_list table_expression access_type optional_lock_
}
//pop the last element which was pushed when we eneterd a new select
SqlParser_CurrentParser->popHasTDFunctions();
-
+ #if 0
--- End diff --
Was this meant to be deleted?
---
[GitHub] trafodion pull request #1688: [TRAFODION-3034] Support Oracle Hierarchy Quer...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1688#discussion_r221817891
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13280,13 +13288,130 @@ table_expression : from_clause where_clause sample_clause
SqlParser_CurrentParser->topHasOlapFunctions());
SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
}
+ | from_clause startwith_clause where_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
+ {
+ $$ =
+ getTableExpressionRelExpr($1,
+ $3,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ }
+ else
+ $$ =
+ getTableExpressionRelExpr($1,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
+ ((BiConnectBy*)$2)->where_clause = $3;
+ $$->setBiConnectBy( $2);
+ $$->setHasConnectByFlag(TRUE);
+ }
+ | from_clause TOK_WHERE search_condition startwith_clause
+ {
+ if($1->getOperatorType() == REL_JOIN)
+ $$ =
+ getTableExpressionRelExpr($1,
+ $3,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ else
+ $$ =
+ getTableExpressionRelExpr($1,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ FALSE,
+ SqlParser_CurrentParser->topHasOlapFunctions());
+ SqlParser_CurrentParser->setTopHasTDFunctions(FALSE);
+ ((BiConnectBy*)$4)->where_clause = $3;
+ //((BiConnectBy*)$3)->order_siblings_by_clause = $4;
+ $$->setBiConnectBy( $4);
+ $$->setHasConnectByFlag(TRUE);
+ }
/* type relx */
from_clause : TOK_FROM global_hint table_reference { $$ = $3; }
| from_clause ',' table_reference
{
$$ = new (PARSERHEAP()) Join($1, $3, REL_JOIN);
- }
+ }
+startwith_clause :TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY search_condition
+ {
+ $$ = new (PARSERHEAP())BiConnectBy ((BiRelat*)$2, (BiRelat*)$5);
+ //save the predicate text
+ $2->unparse(((BiConnectBy*)$$)->startWithString_, PARSER_PHASE, USER_FORMAT);
+ }
+ |TOK_START_WITH search_condition CONNECT_IDENTIFIER TOK_BY TOK_NOCYCLE search_condition
--- End diff --
Could this rule be merged with the previous by adding an optional_nocycle rule? This rule will have 2 possibilities, one of them being "empty". Maybe doing it this way will increase conflicts?
---