Skip to content

Conversation

KyongSik-Yoon
Copy link
Contributor

Parsing failed when sql contain MySql hint SQL_CALC_FOUND_ROWS so I added rule for parsing that.
I referred following documents.
https://dev.mysql.com/doc/refman/5.7/en/select.html

@coveralls
Copy link

coveralls commented Aug 28, 2017

Coverage Status

Coverage increased (+0.02%) to 87.001% when pulling 90d26ee on KyongSik-Yoon:master into 6a440ba on JSQLParser:master.

@coveralls
Copy link

coveralls commented Aug 28, 2017

Coverage Status

Coverage increased (+0.01%) to 86.999% when pulling 8cc0f48 on KyongSik-Yoon:master into 6a440ba on JSQLParser:master.

@coveralls
Copy link

coveralls commented Aug 28, 2017

Coverage Status

Coverage increased (+0.01%) to 86.999% when pulling 4b6f16f on KyongSik-Yoon:master into 6a440ba on JSQLParser:master.

@KyongSik-Yoon KyongSik-Yoon changed the title Supporting MySql hint SQL_CALC_FOUND_ROWS for selecting row count. Implements #509 Aug 31, 2017
public class MySqlSqlCalcFoundRowsTest {
@Test
public void testPossibleParsingWithSqlCalcFoundRowsHint() throws JSQLParserException {
AtomicReference<MySqlSqlCalcFoundRows> ref = new AtomicReference<MySqlSqlCalcFoundRows>();
Copy link
Contributor

Choose a reason for hiding this comment

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

do you really need it to be an atomic reference? could you just assign it to a variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will do that.

@@ -0,0 +1,54 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

could both of these live in just your test class? (otherwise the name might throw people off since it is quite generic)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's created only for unit test.
I'll change the class name to containing means of test.

package net.sf.jsqlparser.expression;

/**
* @author sam
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the reference link here? And that this object supports: SQL_CALC_FOUND_ROWS for the MySQL driver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. SQL_CALC_FOUND_ROWS keyword is hint syntax only for MySQL driver.
You can find here.
https://dev.mysql.com/doc/refman/5.7/en/select.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Mind adding this to the doc:

Supports the {@code SQL_CALC_FOUND_ROWS} keyword for <a href="https://dev.mysql.com/doc/refman/5.7/en/select.html">MySQL driver.</a>

Just so there's a reference in case future devs want to know more about what this does (then they don't have to track this PR down) :)

…odify field type to boolean for prevent memory consumption by creating object and try assertSqlCanBeParsedAndDeparsed on unit test.
@coveralls
Copy link

coveralls commented Oct 19, 2017

Coverage Status

Changes Unknown when pulling 11b62e1 on KyongSik-Yoon:master into ** on JSQLParser:master**.

Copy link
Contributor

@AnEmortalKid AnEmortalKid left a comment

Choose a reason for hiding this comment

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

This looks fine to me 👍 (i'd love if you make that one last javadoc update)

all up to @wumpz !

@wumpz wumpz merged commit 3e16345 into JSQLParser:master Oct 20, 2017
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.

4 participants