mihaibudiu commented on code in PR #4665:
URL: https://github.com/apache/calcite/pull/4665#discussion_r2590784103
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -168,6 +168,21 @@ public class ${parser.class} extends SqlAbstractParserImpl
private Casing quotedCasing;
private int identifierMaxLength;
private SqlConformance conformance;
+ private int rowValueStarCount;
+
+ private void pushRowValueStar() {
+ rowValueStarCount++;
+ }
+
+ private void popRowValueStar() {
+ if (rowValueStarCount > 0) {
Review Comment:
Can this be false?
If not, maybe this can be an assert.
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4276,6 +4302,9 @@ SqlNode AtomicRowExpression() :
e = ContextVariable()
|
e = CompoundIdentifier()
+ |
Review Comment:
The natural question that follows is about `ROW(T.*)`. But that can be part
of a follow-up PR, including perhaps `ROW(T.* EXCLUDE(list))`
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -4276,6 +4302,9 @@ SqlNode AtomicRowExpression() :
e = ContextVariable()
|
e = CompoundIdentifier()
+ |
+ LOOKAHEAD({ allowRowValueStar() })
Review Comment:
didn't know that LOOKAHEAD allows a function call
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7279,11 +7294,45 @@ public static boolean isAmbiguousException(Exception
ex) {
CallCopyingArgHandler argHandler =
new CallCopyingArgHandler(call, false);
call.getOperator().acceptCall(this, call, true, argHandler);
- final SqlNode result = argHandler.result();
+ final SqlNode result = expandRow(argHandler.result());
validator.setOriginal(result, call);
return result;
}
+ private SqlNode expandRow(SqlNode node) {
+ if (!(node instanceof SqlCall)) {
+ return node;
+ }
+ final SqlCall call = (SqlCall) node;
+ if (call.getKind() != SqlKind.ROW) {
+ return node;
+ }
+ final SqlValidatorScope scope = getScope();
+ if (!(scope instanceof SelectScope)) {
Review Comment:
what happens if the scope is not Select? Will eventually an error be
produced?
Or is this the right place to produce the error?
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -766,6 +766,21 @@ private boolean expandStar(List<SqlNode> selectItems,
Set<String> aliases,
}
}
+ private List<SqlNode> expandRowStar(SqlIdentifier identifier, SelectScope
scope) {
Review Comment:
Once we merge #4662 this should probably be enhanced to support EXCLUDE list
in ROW(*) as well.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -7279,11 +7294,45 @@ public static boolean isAmbiguousException(Exception
ex) {
CallCopyingArgHandler argHandler =
new CallCopyingArgHandler(call, false);
call.getOperator().acceptCall(this, call, true, argHandler);
- final SqlNode result = argHandler.result();
+ final SqlNode result = expandRow(argHandler.result());
Review Comment:
I find is a bit weird to have expandRow happen as a side-effect of
calculatePermuteOffset; maybe this is a sign that the function name should be
updated.
##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -3925,27 +3940,38 @@ SqlNode Expression3(ExprContext exprContext) :
LOOKAHEAD(3)
<ROW> {
s = span();
+ pushRowValueStar();
Review Comment:
good point about nested ROW constructors.
I wonder what spark does.
It seems that `ROW(*, ROW(*))` is legit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
