zstan commented on code in PR #13464:
URL: https://github.com/apache/ignite/pull/13464#discussion_r3775069783
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java:
##########
@@ -267,10 +268,65 @@ private void validateTableModify(SqlNode table) {
@Override protected void validateSelect(SqlSelect select, RelDataType
targetRowType) {
super.validateSelect(select, targetRowType);
- validateFetchOffset(select.getFetch(), "fetch / limit");
+ validateFetch(select, "fetch / limit");
validateFetchOffset(select.getOffset(), "offset");
}
+ /** Validate fetch expression. */
+ // TODO: https://issues.apache.org/jira/browse/CALCITE-7592
+ // Remove this method after upgrading to Calcite 1.43.
+ private void validateFetch(SqlSelect select, String clauseName) {
+ SqlNode fetch = select.getFetch();
+
+ if (fetch == null)
+ return;
+
+ if (SqlUtil.isNullLiteral(fetch, true))
+ throw newValidationError(fetch,
IgniteResource.INSTANCE.illegalFetchLimit(clauseName));
+
+ validateFetchExpression(fetch, clauseName);
+ deriveDynamicParameterTypes(fetch);
+
+ RelDataType type = deriveType(getWhereScope(select), fetch);
+
+ if (type.getSqlTypeName().getFamily() != SqlTypeFamily.NUMERIC)
+ throw newValidationError(fetch,
IgniteResource.INSTANCE.illegalFetchLimit(clauseName));
+
+ validateFetchOffset(fetch, clauseName);
+ }
+
+ /** Reject column references, aggregate functions, and window functions in
a fetch expression. */
+ // TODO: https://issues.apache.org/jira/browse/CALCITE-7592
+ // Remove this method after upgrading to Calcite 1.43.
+ private void validateFetchExpression(SqlNode node, String clauseName) {
+ if (node instanceof SqlIdentifier) {
+ if (makeNullaryCall((SqlIdentifier)node) == null)
+ throw newValidationError(node,
IgniteResource.INSTANCE.illegalFetchLimit(clauseName));
+
+ return;
+ }
+
+ if (node instanceof SqlNodeList) {
+ for (SqlNode child : (SqlNodeList)node)
+ validateFetchExpression(child, clauseName);
+ }
+ else if (node instanceof SqlCall call) {
+ if (call.isA(SqlKind.QUERY))
+ throw newValidationError(call,
IgniteResource.INSTANCE.illegalFetchLimit(clauseName));
+
+ if (call.getKind() == SqlKind.OVER)
Review Comment:
I don\`t think that this is a good approach:
1. comment this line : if (call.getKind() == SqlKind.OVER)
and run 'testInvalidFetchExpression' - you obtain aggregate related error, i
mean - if you miss some operands comparison here - you will obtain confusing
exception.
2. I don\`t like an idea to handle each stupid oparation here, you will
always miss smth, check :
'SELECT * FROM TEST_REPL FETCH FIRST (1=1) ROWS ONLY' but if you want - you
can do it with extended tests for all such a cases
--
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]
