Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: in prepared statement limit/offset may be placeholder #2364

Merged
merged 2 commits into from
Jan 3, 2017

Conversation

shenli
Copy link
Member

@shenli shenli commented Dec 31, 2016

Fix #2180
In prepareed statement, limit/offset may be placeholder:
prepare stmt_test_1 from 'select id from prepare_test limit ? offset ?'

err error
)
if limit.Offset != nil {
offset, err = getUintForLimitOffset(limit.Offset.GetValue())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using evalAstExpr?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expr must be ConstExpr and its real value must be unsigned int.

@coocood
Copy link
Member

coocood commented Jan 3, 2017

LGTM

@shenli
Copy link
Member Author

shenli commented Jan 3, 2017

@hanfei1991 PTAL

@shenli shenli changed the title *: in prepareed statement limit/offset may be placeholder *: in prepared statement limit/offset may be placeholder Jan 3, 2017
@hanfei1991
Copy link
Member

hanfei1991 commented Jan 3, 2017

mysql> prepare ppp from "select * from t limit ?";

mysql> set @A = 1.1;
Query OK, 0 rows affected (0.00 sec)

mysql> execute ppp using @A;
+------+------+
| c1 | c2 |
+------+------+
| 1 | 1 |
+------+------+
1 row in set (0.00 sec)

double is also ok. May be we should build a datum and cast it to int.

@shenli
Copy link
Member Author

shenli commented Jan 3, 2017

@hanfei1991 PTAL

Copy link
Member

@hanfei1991 hanfei1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shenli shenli merged commit d40e24c into master Jan 3, 2017
@shenli shenli deleted the shenli/issue-2180 branch January 3, 2017 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants