Re: [I] Problem with limit '2'::int [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu commented on issue #34976:
URL: https://github.com/apache/shardingsphere/issues/34976#issuecomment-3583950296

   ### Understanding
   
     - You want to know whether master supports limit '2'::int and note that placeholders in LIMIT need proper handling during rewrite.
   
     ### Root Cause
   
     - In PostgreSQL/openGauss visitors, visitSelectLimitValue/visitSelectOffsetValue/visitSelectFetchValue call Long.parseLong(((ExpressionSegment) astNode).getText()), accepting only plain numbers or bare parameters.
     - When LIMIT uses type casts or simple expressions ('2'::int, ?::int), getText() is not a pure number, so parsing throws an exception before rewrite.
     - A placeholder wrapped in a cast is not recognized as a ParameterMarkerExpressionSegment, so its index is lost and rewrite cannot bind it correctly.
   
     ### Analysis (code-level fix direction)
   
     - Update both PostgreSQL and openGauss visitors to unwrap the expression tree, preserve parameters, and fold constants instead of blindly parseLong.
     - Suggested helper (add to both visitors; can be a shared private method):
   
   ```java
       private LimitValueSegment toLimitValue(final ParserRuleContext ctx, final ExpressionSegment segment) {
           if (segment instanceof ParameterMarkerExpressionSegment) {
               return new ParameterMarkerLimitValueSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(),
                       ((ParameterMarkerExpressionSegment) segment).getParameterMarkerIndex());
           }
           if (segment instanceof TypeCastExpression) {
               return toLimitValue(ctx, ((TypeCastExpression) segment).getExpression());
           }
           if (segment instanceof LiteralExpressionSegment) {
               Object literals = ((LiteralExpressionSegment) segment).getLiterals();
               return new NumberLiteralLimitValueSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(),
                       Long.parseLong(literals.toString()));
           }
           // Optionally extend to UnaryExpression / BinaryOperationExpression constant folding
           throw new SQLParsingException("Unsupported LIMIT expression: " + segment.getText());
       }
   
       @Override
       public ASTNode visitSelectLimitValue(final SelectLimitValueContext ctx) {
           if (null != ctx.ALL()) {
               return null;
           }
           ExpressionSegment expr = (ExpressionSegment) visit(ctx.cExpr());
           return toLimitValue(ctx, expr);
       }
   ```
   
     - Key points:
         - Preserve parameters: ?::int should become ParameterMarkerLimitValueSegment; the index remains intact for rewrite.
         - Type casts/constant expressions: allow '2'::int to fold into a numeric literal, avoiding parse errors.
         - Other complex expressions: throw a clear “unsupported LIMIT expression” error rather than a NumberFormatException.
     - Tests to add:
         - PostgreSQL/openGauss parser UTs for limit '2'::int, limit ?::int, limit '2'::int offset '1'::int, asserting LimitSegment rowCount/offset types and values (numeric vs parameter index).
         - Optional SQL rewrite UT ensuring limit ?::int keeps the parameter list intact and the placeholder count/order unchanged.
   
     ### Conclusion
   
     - Master still has this gap; it only supports plain numbers or bare parameters in LIMIT.
     - Community contributors are warmly invited to submit a PR implementing the above parsing changes and tests—we’ll gladly review and help land it.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org