zstan commented on code in PR #13464:
URL: https://github.com/apache/ignite/pull/13464#discussion_r3774926160
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -1072,6 +1077,11 @@ private long validateAndGetFetchOffsetParams(RexNode
node, String op) {
Supplier<Object> scalar = expressionFactory.execute(node);
Object param = scalar.get();
+ if (param == null && !(node instanceof RexDynamicParam)) {
Review Comment:
'node instanceof RexDynamicParam' - looks like redundant check ? Or plz show
me appropriate test ?
##########
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");
Review Comment:
now we have 2 functions: one - for 'validateFetch' and other for
'validateFetchOffset' ? looks weird
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -655,6 +656,10 @@ private boolean hasExchange(RelNode rel) {
long offset = validateAndGetOffset(rel.offset,
SortNode.OFFSET_DEFAULT);
long fetch = validateAndGetFetch(rel.fetch, SortNode.FETCH_DEFAULT);
+ // Zero FETCH is enforced by the outer IgniteLimit, while SortNode
accepts only positive FETCH values.
+ if (fetch == 0)
Review Comment:
why do we need this change ? I run limited set of tests and it\`s all ok,
plz clarify ?
Also if this fix is helpful for smth. it looks hacky
1. run validateAndGetFetch with SortNode.FETCH_DEFAULT
2. additional check and additional set : fetch = SortNode.FETCH_DEFAULT;
--
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]
