SQL builder: add subclass support

[mattsgarlata]

I'd like to be able to subclass the SQL class so I can add custom functionality to it. The problem is, most of the stuff inside AbstractSQL is private, so I can't modify the things I want to modify. It would be great if those could be protected instead. I would be happy to submit a pull request, if you agree this would be a worthwhile change.

FWIW, the functionality I want to add is the ability to put in a TOP or LIMIT clause to limit the number of returned results.

PS - I'm a big fan of how the SQL class works. I wrote something similar in PL/SQL years ago, but never could figure out a Java syntax I was happy with.

[h3adache]

Duplicate of #904

[h3adache]

Oops sorry this is different. This is about extending the SQL class.

[h3adache]

Does this not work? sql.SELECT("TOP 10 x").FROM("foo")

[mattsgarlata]

That would work for TOP, but I think LIMIT is more complicated. The syntax for LIMIT (used by MySQL and HyperSQL) is:

select *
from foo
limit 1000

[mattsgarlata]

FWIW, it looks like basically every database is a special snowflake for this functionality.

https://en.wikipedia.org/wiki/Select_(SQL)#FETCH_FIRST_clause

It looks like Firebird also introduces a custom clause like LIMIT. It's called ROWS for them.

In my subclass I was planning on adding a method that would look at the database type and then choose where to insert the appropriate SQL. Right now the SQL Builder is database agnostic, so this might be more appropriate in my code than in iBatis.

[h3adache]

Yea it is. LIMIT in SQLServer for example works just like TOP. It would be awkward (and not preferable) to add that type of logic into mybatis. Since the SQL class just returns a string, it's conceivable to just manipulate that string directly. To handle it 'generically' in the SQL builder, we could expose general setters that allow you to append your own String but that might but confusing. Something like that would possibly be necessary should we support #904 though. I'm open to suggestions on 'cleaner' ways to handle this.

[mattsgarlata]

I've already actually spent some time trying to manipulate SQL strings, and it's pretty messy.

I don't think it would be advisable to allow adding generic Strings to the SQL statement. The big problem is: where do those Strings go? As you mentioned, it would also be confusing for people who are just getting started with the SQL class.

I think the simplest thing is to just change everything in SQL and AbstractSQL from private to protected. That keeps the public API equally clean as it was before, but now opens up a subclass to handle any crazy database-specific stuff that it wants.

[h3adache]

I think a better solution would be to find a way to accommodate this #903 and #904 . I can think of a solution but I'm confused on a comment in a previous commit and will need to bring this up with the other committers.

[jeffgbutler]

At the risk of making this go down a rabbit hole, I'd like to mention that I've been working on a SQL templating library that has some enhanced support for MyBatis. It's original design goal is to make the code coming out of the generator better and more flexible, but I think it has a use as a general SQL generator too.

I'd be interested to know if you think it could be easily extended to do what you want.

https://github.com/jeffgbutler/mybatis-dynamic-sql

[mattsgarlata]

For me personally, I'm hoping to stay as close to the SQL as possible. I'd prefer not to have another layer of abstraction I need to think about, and I'd like to stay close to the raw SQL so I have the power to do whatever I need.

The only reason I'm not writing the SQL by hand is I need to have it build up piece by piece to do complex reporting, which wouldn't really be practical with just String manipulation.

[h3adache]

The problem with exposing those classes is that we then have to consider them when refactoring due to backwards compatibility. Fixing #903 is easily done in the code and #904 should probably better be handled in a library like the one that @jeffgbutler is working on dedicated to handling sql generation. For your issue all the syntaxes mentioned before can be 'solved' using various forms of

sql.SELECT("TOP 10 x").FROM("foo") sql.SELECT("x").FROM("foo LIMIT 10")

As they fall under "Non-standard syntax" anyways.

Fetch first that you mentioned is interesting because it's a SQL 2008 standard and supported by a lot of major db vendors.

SELECT * FROM T FETCH FIRST 10 ROWS ONLY We can support that using: sql.SELECT("x").FROM("foo").WHERE("WHERE ID_T > 10 FETCH FIRST 10 ROWS ONLY") Or are you asking for something more like sql.LIMIT(xxx) or sql.FETCH_FIRST(xxx)?

[mattsgarlata]

FETCH FIRST isn't supported by MySQL, so at least for now it wouldn't solve my problem.

It's an interesting idea to tack on TOP 10 to the from clause, but adding LIMIT to the where clause wouldn't work for me. Sometimes my queries won't have a WHERE. I could conditionally add to the FROM clause instead, but that quickly becomes messy and kind of defeats the purpose of using the SQL builder in the first place.

I was hoping to add something like sql.LIMIT(xxx) or sql.FETCH_FIRST(xxx) within my own custom subclass that would handle adding the LIMIT or whatever database specific statement was required in the appropriate place. (I think FETCH_FIRST would be a better name since that will be the standard in the future)

FWIW, to get the type of the database you can call connnection.getMetaData().getDatabaseProductName(). Example values for that are:

Oracle
MySQL
Microsoft SQL Server
ACCESS
HSQL Database Engine

I was planning on passing in the result of that into the constructor for my custom subclass of SQL, and then in the FETCH_FIRST method, branch on that.

If you were interested in incorporating functionality like this into iBatis itself, you could always make the constructor with the type of database optional. This new functionality would only be available if the new constructor was used, and backward compatibility would be maintained. If someone tried to use the FETCH_FIRST method without the new constructor, it could just fall back to using the FETCH FIRST standard SQL syntax.

[mattsgarlata]

Another reason I was hoping to have this was so that I could implement a clone method to do a deep clone of the SQL class. For what I'm working on, I need to run a bunch of queries with similar where clauses. I'd like to build up my where and then clone it whenever I need a copy of it. Instead right now I have to build the where clause over and over again in new SQL instances.