[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user asfgit closed the pull request at: https://github.com/apache/incubator-trafodion/pull/577 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71197478
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6953,13 +6968,22 @@ del_stmt_w_acc_type_rtn_list_and_as_clause_col_list
: '(' delete_statement acce
table_name_as_clause_and_hint : table_name as_clause optimizer_hint
hbase_access_options
{
- $1->setCorrName(*$2);
- $$ = new (PARSERHEAP()) Scan(*$1);
- if ($3)
-$$->setHint($3);
- if ($4)
-((Scan*)$$)->setHbaseAccessOptions($4);
-
+ NAString tmp =
((*$1).getQualifiedNameAsString());
+
if(SqlParser_CurrentParser->hasWithDefinition(&tmp) )
+ {
+ RelExpr *re =
SqlParser_CurrentParser->getWithDefinition(&tmp);
+ RenameTable *rt = new (PARSERHEAP())
RenameTable(re, *$2);
+ $$=rt->copyTree(PARSERHEAP());
--- End diff --
Same as above, could give an error if we see a hint or an HBase access
option.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71197264
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6647,15 +6651,26 @@ table_as_tmudf_function : TOK_UDF '('
table_mapping_function_invocation ')'
table_name_and_hint : table_name optimizer_hint hbase_access_options
{
-$$ = new (PARSERHEAP()) Scan(*$1);
-if ($2)
- $$->setHint($2);
+NAString tmp = ((*$1).getQualifiedNameAsString());
+
if(SqlParser_CurrentParser->hasWithDefinition(&tmp) )
+{
+ RelExpr *re =
SqlParser_CurrentParser->getWithDefinition(&tmp);
+ $$=re->copyTree(PARSERHEAP());
+ delete $1;
--- End diff --
One thing we could do here is to raise an error if we have an optimizer
hint or HBase access options at this point. Those are allowed in the grammar,
but we wouldn't be able to apply them to a more complicated WITH clause.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71083286
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
sorry, I made mistake, by running TESTOK2, it shows the change still
introduce one more reduce/reduce conflict. Still looking at 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71058116
--- Diff: core/sql/parser/sqlparser.y ---
@@ -7000,6 +7024,46 @@ rel_subquery_and_as_clause : rel_subquery as_clause
CheckModeSpecial1();
$$ = $2;
}
+with_clause :
+ TOK_WITH with_clause_elements
+ {
+ $$ = NULL;
+ }
+ | TOK_WITH TOK_RECURSIVE with_clause_elements
+ {
+*SqlParser_Diags << DgSqlCode(-3022) <<
DgString0("WITH RECURSIVE");
+YYERROR;
+ }
+
+with_clause_elements : with_clause_element
+ {
+ $$ = NULL;
+ }
+ | with_clause_elements ',' with_clause_element
+ {
+ $$ = NULL;
+ }
+
+with_clause_element : correlation_name TOK_AS '(' query_expression ')'
+ {
+ RelRoot *root = new (PARSERHEAP())
+RelRoot($4, REL_ROOT);
+ $$= new (PARSERHEAP()) RenameTable(root, *$1);
+
+ //Duplicated definition of WITH
+ if(SqlParser_CurrentParser->hasWithDefinition($1)
)
+ {
+*SqlParser_Diags << DgSqlCode(-3288)
+ <<
DgString0((*$1).toCharStar());
+ delete $1;
+ delete $4;
+ YYERROR;
+ }
+
+ SqlParser_CurrentParser->insertWithDefinition($1
, $$);
+ delete $1;
+ delete $4;
--- End diff --
I agree, will change 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71058095
--- Diff: core/sql/parser/sqlparser.y ---
@@ -1,14 +13396,41 @@ query_specification : select_token set_quantifier
query_spec_body
ColReference(new (PARSERHEAP())
ColRefName(TRUE, PARSERHEAP()))
)
);
- assert($3->getOperatorType() == REL_ROOT);
+ assert($3->getOperatorType() == REL_ROOT);
+ RelRoot *root1 = (RelRoot *) $$;
+ RelRoot *root2 = (RelRoot *) $3;
+ root1->assignmentStTree() = root2->assignmentStTree();
+ root2->assignmentStTree() = NULL;
+ }
+ else
+ $$ = $3;
+
+ if (CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) == DF_DUMP ||
+ CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) == DF_DUMP_MV)
+ ((RelRoot*)$$)->setAnalyzeOnly();
+
+}
+
+/* type relx */
+query_specification :with_clause select_token set_quantifier
query_spec_body
+ {
+ if ($3) {
+ $$ = new (PARSERHEAP())
+ RelRoot(new (PARSERHEAP())
+ GroupByAgg($4
--- End diff --
if ($3) means there is a set_quantifier token, which is DISTINCT for
example, so need a GroupByAgg node here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71057841
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13418,6 +13507,55 @@ query_specification : select_token '['
firstn_sorted NUMERIC_LITERAL_EXACT_NO_SC
if (CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) == DF_DUMP
||
CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) ==
DF_DUMP_MV)
((RelRoot*)$$)->setAnalyzeOnly();
+ }
+/* type relx */
+query_specification : with_clause select_token '[' firstn_sorted
NUMERIC_LITERAL_EXACT_NO_SCALE ']' set_quantifier query_spec_body
+ {
+ if ($7) {
+ $$ = new (PARSERHEAP())
+ RelRoot(new (PARSERHEAP())
+ GroupByAgg($8
--- End diff --
if($7) means there is a set_quantifier like (DISTINCT), so it will add a
GroupByAgg node here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71010941
--- Diff: core/sql/parser/sqlparser.y ---
@@ -7000,6 +7024,46 @@ rel_subquery_and_as_clause : rel_subquery as_clause
CheckModeSpecial1();
$$ = $2;
}
+with_clause :
+ TOK_WITH with_clause_elements
+ {
+ $$ = NULL;
+ }
+ | TOK_WITH TOK_RECURSIVE with_clause_elements
+ {
+*SqlParser_Diags << DgSqlCode(-3022) <<
DgString0("WITH RECURSIVE");
+YYERROR;
+ }
+
+with_clause_elements : with_clause_element
+ {
+ $$ = NULL;
+ }
+ | with_clause_elements ',' with_clause_element
+ {
+ $$ = NULL;
+ }
+
+with_clause_element : correlation_name TOK_AS '(' query_expression ')'
+ {
+ RelRoot *root = new (PARSERHEAP())
+RelRoot($4, REL_ROOT);
+ $$= new (PARSERHEAP()) RenameTable(root, *$1);
+
+ //Duplicated definition of WITH
+ if(SqlParser_CurrentParser->hasWithDefinition($1)
)
+ {
+*SqlParser_Diags << DgSqlCode(-3288)
+ <<
DgString0((*$1).toCharStar());
+ delete $1;
+ delete $4;
+ YYERROR;
+ }
+
+ SqlParser_CurrentParser->insertWithDefinition($1
, $$);
+ delete $1;
+ delete $4;
--- End diff --
I don't think we want to delete $1 and $4 here, since the RelRoot node
points to $4 and the RenameTable node points to $1.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71010585
--- Diff: core/sql/parser/sqlparser.y ---
@@ -13418,6 +13507,55 @@ query_specification : select_token '['
firstn_sorted NUMERIC_LITERAL_EXACT_NO_SC
if (CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) == DF_DUMP
||
CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) ==
DF_DUMP_MV)
((RelRoot*)$$)->setAnalyzeOnly();
+ }
+/* type relx */
+query_specification : with_clause select_token '[' firstn_sorted
NUMERIC_LITERAL_EXACT_NO_SCALE ']' set_quantifier query_spec_body
+ {
+ if ($7) {
+ $$ = new (PARSERHEAP())
+ RelRoot(new (PARSERHEAP())
+ GroupByAgg($8
--- End diff --
I don't understand why we add a GroupByAgg node here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r71010430
--- Diff: core/sql/parser/sqlparser.y ---
@@ -1,14 +13396,41 @@ query_specification : select_token set_quantifier
query_spec_body
ColReference(new (PARSERHEAP())
ColRefName(TRUE, PARSERHEAP()))
)
);
- assert($3->getOperatorType() == REL_ROOT);
+ assert($3->getOperatorType() == REL_ROOT);
+ RelRoot *root1 = (RelRoot *) $$;
+ RelRoot *root2 = (RelRoot *) $3;
+ root1->assignmentStTree() = root2->assignmentStTree();
+ root2->assignmentStTree() = NULL;
+ }
+ else
+ $$ = $3;
+
+ if (CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) == DF_DUMP ||
+ CmpCommon::getDefault(MVQR_LOG_QUERY_DESCRIPTORS) == DF_DUMP_MV)
+ ((RelRoot*)$$)->setAnalyzeOnly();
+
+}
+
+/* type relx */
+query_specification :with_clause select_token set_quantifier
query_spec_body
+ {
+ if ($3) {
+ $$ = new (PARSERHEAP())
+ RelRoot(new (PARSERHEAP())
+ GroupByAgg($4
--- End diff --
I don't understand why we stick a GroupByAgg node here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70897000
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
Thanks, that's excellent news!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70756198
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6647,6 +6652,26 @@ table_as_tmudf_function : TOK_UDF '('
table_mapping_function_invocation ')'
table_name_and_hint : table_name optimizer_hint hbase_access_options
{
+NAString tmp = ((*$1).getQualifiedNameAsString());
+
if(SqlParser_CurrentParser->with_clauses_->contains(&tmp) )
+{
+ RelExpr *re =
SqlParser_CurrentParser->with_clauses_->getFirstValue(&tmp);
+ $$=re->copyTree(PARSERHEAP());
--- End diff --
yes, I am not quite sure how memory was managed in the parser, I changed
to delete $1
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request: https://github.com/apache/incubator-trafodion/pull/577#discussion_r70756042 --- Diff: core/sql/bin/SqlciErrors.txt --- @@ -410,6 +410,7 @@ 1430 3F000 9 BEGINNER MAJOR DBADMIN A schema name that starts and ends with an "_"(underscore) is reserved for internal usage. It cannot be used to create a user schema. 1431 Z 9 BEGINNER MINOR DBADMIN Object $0~String0 exists in HBase. This could be due to a concurrent transactional ddl operation in progress on this table. 1432 Z 9 BEGINNER MINOR DBADMIN Input LOB type $0~Int0 does not match column's storage type: $1~Int1 Column name: $0~String0 . +1433 Z 9 BEGINNER MINOR DBADMIN WITH clause redefined. WITH name $0~String0 . --- End diff -- yes, changed to 3288 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70755998
--- Diff: core/sql/sqlcomp/parser.h ---
@@ -261,6 +261,8 @@ ItemExpr *get_w_ItemExprTree(const NAWchar * str,
NABoolean isHQCCacheable()
{ return HQCKey_?HQCKey_->isCacheable():FALSE; }
+ NAHashDictionary *with_clauses_;
--- End diff --
yes, changed
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70736959
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
I found a way to avoid the reduce/shift and reduce/reduce error by add
with_clause down to the underlying rules of non_join_query_expression, instead
of add it at same level of non_join_query_expression. non_join_query_expression
can represent many different styles of query expression, including utilities,
which we don't want a with_clause as prefix, so in the underlying rules, only
add with_clause before those SELECT syntax, and it solve the problem.
And it seems if use redundant rule explicitly (looks annoying) , we can
avoid the error.
For example:
A : b | c
is better to change to
A : b
A : c
I am still trying to understand this.
I checked with PostgreSQL parser, they are doing similar things, comments
from postgresql parser:
* This rule parses the equivalent of the standard's .
* The duplicative productions are annoying, but hard to get rid of without
* creating shift/reduce conflicts.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70656018
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
Yes, I got a similar result. I tried this:
query_expression : with_clause non_join_query_expression
| non_join_query_expression
That gives me three additional reduce/reduce conflicts which I don't
understand. Here are two of them:
State 1941
1870 query_expression: with_clause non_join_query_expression .
1871 | non_join_query_expression .
TOK_EXCEPT reduce using rule 1870 (query_expression)
TOK_EXCEPT [reduce using rule 1871 (query_expression)]
TOK_UNION reduce using rule 1870 (query_expression)
TOK_UNION [reduce using rule 1871 (query_expression)]
$defaultreduce using rule 1870 (query_expression)
I also tried this:
query_expression : optional_with_clause non_join_query_expression
That doesn't cause additional reduce/reduce conflicts, but we get a very
large number of additional shift/reduce conflicts, something in the
neighborhood of 50-100.
If we can't get the "correct" parser rules to work, then maybe we can stick
with the approach you chose (assuming it accepts all the valid WITH clauses)
and handle any illegal syntax in the semantic actions.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70620967
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
hi, Hans, Dave,
the current change doesn't get higher number of conflicts, it is still
73/12, but if I tried to move the rule into query_expression, it will increase
the shift/reduce conflicts, I still cannot find a good way to write a correct
syntax. It is rather difficult to find the root cause of shift/reduce conflict
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70517961
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6579,6 +6581,9 @@ table_reference : table_name_and_hint
| rel_subquery_and_as_clause
{ $$ = $1; }
+ | with_clause_list
--- End diff --
I agree with Hans; it would be better to move the wish_clause non-terminal
to the query_expression production. Right now, that production reads:
query_expression : non_join_query_expression
You could change it to:
query_expression: with_clause non_join_query_expression
| non_join_query_expression,
or alternatively,
query_expression : with_clause non_join_query_expression
And then code with_clause to allow an empty production, for example:
with_clause : { $$ = NULL; /* empty with_clause case */ }
| TOK_WITH with_clause_elements
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70514345
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6579,6 +6581,9 @@ table_reference : table_name_and_hint
| rel_subquery_and_as_clause
{ $$ = $1; }
+ | with_clause_list
--- End diff --
I think this is incorrect. Making "with_clause_list" an alternative for
table_reference would allow things like: "select * from with c1 as (select *
from t)".
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70505167
--- Diff: core/sql/parser/sqlparser.y ---
@@ -7001,6 +7036,46 @@ rel_subquery_and_as_clause : rel_subquery as_clause
$$ = $2;
}
+with_clause_list : with_clause
+ { $$ = $1 ; }
+
+ | with_clause_list ',' correlation_name TOK_AS
rel_subquery
--- End diff --
One way would be:
with_clause : WITH with_clause_elements
with_clause_elements : with_clause element
|
with_clause_elements
with_clause_element
with_clause_element : correlation_name TOK_AS rel_subquery
By the way, I notice the way this is coded, the "with_clause" non-terminal
is a single correlation_name TOK_AS rel_subquery element. It seems more natural
(to me anyway) to think of the with_clause as being everything from the WITH
keyword through the entire list of subqueries.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70504097
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6647,6 +6652,26 @@ table_as_tmudf_function : TOK_UDF '('
table_mapping_function_invocation ')'
table_name_and_hint : table_name optimizer_hint hbase_access_options
{
+NAString tmp = ((*$1).getQualifiedNameAsString());
+
if(SqlParser_CurrentParser->with_clauses_->contains(&tmp) )
+{
+ RelExpr *re =
SqlParser_CurrentParser->with_clauses_->getFirstValue(&tmp);
+ $$=re->copyTree(PARSERHEAP());
--- End diff --
Should we delete $1 in this code path as well?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r70441662
--- Diff: core/sql/parser/sqlparser.y ---
@@ -7001,6 +7036,46 @@ rel_subquery_and_as_clause : rel_subquery as_clause
$$ = $2;
}
+with_clause_list : with_clause
+ { $$ = $1 ; }
+
+ | with_clause_list ',' correlation_name TOK_AS
rel_subquery
--- End diff --
@zellerh , I cannot figure out how to do this. Could you please give me an
example? I am very new to the parser.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69932659
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
That is the number of conflicts encountered by the bison parser when it
processes file core/sql/parser/sqlparser.y. Here is how I get this information:
```
touch core/sql/parser/sqlparser.y
cd core/sql/nskgmake
gmake -j 4 -ks linuxdebug
```
Here is the current output:
```
../parser/sqlparser.y: warning: 73 shift/reduce conflicts [-Wconflicts-sr]
../parser/sqlparser.y: warning: 12 reduce/reduce conflicts [-Wconflicts-rr]
```
If you get a different (higher) number, then we would have to investigate
whether there are any potential problems with the new syntax.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user traflm commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69848822
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
@zellerh , I don't understand what is "number of shift/reduce and
reduce/reduce conflicts". How to check this number? What is 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request: https://github.com/apache/incubator-trafodion/pull/577#discussion_r69839237 --- Diff: core/sql/bin/SqlciErrors.txt --- @@ -410,6 +410,7 @@ 1430 3F000 9 BEGINNER MAJOR DBADMIN A schema name that starts and ends with an "_"(underscore) is reserved for internal usage. It cannot be used to create a user schema. 1431 Z 9 BEGINNER MINOR DBADMIN Object $0~String0 exists in HBase. This could be due to a concurrent transactional ddl operation in progress on this table. 1432 Z 9 BEGINNER MINOR DBADMIN Input LOB type $0~Int0 does not match column's storage type: $1~Int1 Column name: $0~String0 . +1433 Z 9 BEGINNER MINOR DBADMIN WITH clause redefined. WITH name $0~String0 . --- End diff -- Parser errors should be in the range 3000-3999. Also, since this is a syntax error, use 42000 (the SQLSTATE value for syntax errors) instead of "Z". --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69837244
--- Diff: core/sql/sqlcomp/parser.h ---
@@ -261,6 +261,8 @@ ItemExpr *get_w_ItemExprTree(const NAWchar * str,
NABoolean isHQCCacheable()
{ return HQCKey_?HQCKey_->isCacheable():FALSE; }
+ NAHashDictionary *with_clauses_;
--- End diff --
Can you make this a private data member?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69837060
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
Related question: When you made this change, did the number of shift/reduce
and reduce/reduce conflicts change?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69836802
--- Diff: core/sql/parser/sqlparser.y ---
@@ -14684,6 +14759,11 @@ optional_limit_spec : TOK_LIMIT
NUMERIC_LITERAL_EXACT_NO_SCALE
dml_statement : dml_query { $$ = $1; }
+ | with_clause_list dml_query
--- End diff --
Same comment as a above about where the parser rule for the WITH clause
should be.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69836669
--- Diff: core/sql/parser/sqlparser.y ---
@@ -7001,6 +7036,46 @@ rel_subquery_and_as_clause : rel_subquery as_clause
$$ = $2;
}
+with_clause_list : with_clause
+ { $$ = $1 ; }
+
+ | with_clause_list ',' correlation_name TOK_AS
rel_subquery
--- End diff --
Can we do this in a way that doesn't have two different rules for the first
and the subsequent parts of the WITH clause? See comment above for how the ANSI
standard handles the WITH clause.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69836226
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6579,6 +6581,9 @@ table_reference : table_name_and_hint
| rel_subquery_and_as_clause
{ $$ = $1; }
+ | with_clause_list
--- End diff --
Here is the syntax rule from the ANSI standard:
::=
[ ]
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #577: [TRAFODION-1673] support with clause
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/577#discussion_r69835003
--- Diff: core/sql/parser/sqlparser.y ---
@@ -6579,6 +6581,9 @@ table_reference : table_name_and_hint
| rel_subquery_and_as_clause
{ $$ = $1; }
+ | with_clause_list
--- End diff --
The ANSI SQL standard defines the "with" clause as part of a
query_expression, so I would recommend doing the same here
(non_join_query_expression in our case). Also, it would be good to accept the
WITH RECURSIVE syntax and to give an error that this is not yet supported.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
