[CALCITE-5261] UNION(ALL) inside of the CURSOR throws an exception while validating the query#5056
Conversation
| } | ||
| }; | ||
| ns = createSetopNamespace(call, call); | ||
| } else { |
There was a problem hiding this comment.
This part is not obvious to me, and it does not have coverage in the tests. Can it be reached? Could you also reduce the set case in the same way?
There was a problem hiding this comment.
Thanks for the suggestion. Here was reachable and in fact contained a bug: for a query like cursor(values ('a'), ('b')), it would hit the else branch and throw ClassCastException because of the cast (SqlSelect) query.
I refactored the code to handle the non-SELECT cases explicitly:
SELECTkeeps the originalSelectScope+SelectNamespacebehavior.UNION/INTERSECT/EXCEPTnow use aListScope+SetopNamespace.VALUESusesTableConstructorNamespace.WITHusesWithNamespace.- Anything else throws
Util.unexpected(...)(because can not use real sql to cover this condition).
Regarding reducing the set case the same way: I tried using parentScope directly for SET_QUERY, but it fails because parentScope is often a DelegatingScope, whose addChild throws UnsupportedOperationException. So the ListScope maybe still needed.
I also added test coverage for all branches in SqlValidatorTest.testCollectionTableWithCursorParam and a new testDeclareCursorUnexpectedKind.
mihaibudiu
left a comment
There was a problem hiding this comment.
Are there any quidem tests with cursor?
(If so, it would be nice to have one, but maybe it's not supported)
Yes, That was my plan at first, too. But Maybe there is no way to add a quidem test for The reason is that Since quidem is an end-to-end execution test framework:
I verified earlier that So |
…ile validating the query
|
|
Since this pr has been approved, I had squashed the commits. If there are no other comments, I will merge it. |



jira: https://issues.apache.org/jira/browse/CALCITE-5261