Added negative _LIMIT to select backwards!#74
Added negative _LIMIT to select backwards!#74kolchagov wants to merge 3 commits intorestsql:masterfrom
Conversation
|
Appreciate the contribution! I'll be looking at this soon in more detail. From a brief review, seems to Also I'm not clear one the value of context.xml. I've never used them It's also difficult to see in AbstractSqlBuilder.java what has changed in Lastly, you'll need to add yourself to the contributors.txt. And make sure Cheers, On Wed, Jul 20, 2016 at 12:22 PM, kolchagov notifications@github.com
|
|
Hi, |
Views can be used to implement currently unsupported aggregate functions. Fixed default config properties read incorrectly
|
As current version doesn't support aggregate functions, and I saw that the views are explicitly excluded, I've committed code which enables the views and treat them as readOnly tables. The changes are very light and are related only to MySQL's resource generation. |
|
Hello kochagov. Thanks for your patience. Finally found some time this morning to review your work. I looked at your latest message and commit with the enabling of queries on read-only columns, but am puzzled on the intention. Are you submitting this to enable aggregate functions, views or read only columns? View support needs a bit more work. The framework does not properly inspect metadata from views. Just to check if my memory fails me, I created a resource on sakila.actor_info. It doesn't get past the initial query for building the metadata: Definition requires table element for actor, referenced by column id. It probably has to change to read from the info schema, since the result set metadata query is not working. By aggregate functions, I mean UNION, UNION ALL, INTERSECT and MINUS. How does this help with those? By read-only columns, I mean a function, e.g. length(last_name). You can see test cases in restsql-test/resources/xml/service/testcase/ReadOnlyColumns and the resource tested is resrouces/xml/sqlresources/SingleTableFuncCols.xml. MySQL at least does not allow query by column alias. Say the above column was aliased to surname. If the query had a filter - where surname > 10 - it will fail. You have to repeat the function, as in where length(last_name) > 2. restSQL is not smart enough to do that yet. To level-set on practices, I don't accept any changes, even from myself, without insurance. All core framework functionality needs to be covered by examples and tests, preferably positive and negative, in the restsql-test. They need to be run against both MySQL and PostgreSQL. If they don't run against PostgreSQL, then we can put them in the folder that called MySqlOnly and these are excluded in a MySql test run. They also have to be executed in command line mode and at least on app server. I can do this test work, but the timeline is subject to my availability. You're welcome to explore the http://restsql.org/doc/README-FRAMEWORK_DEVELOPERS.txt. Sorry it's not a great doc. And haven't reviewed it in years, so I hope it's up to date. Do let me know what your intentions are with this latest commit and some examples. Thanks, |
|
Hello Mark, |
|
Hi Ivan. The project doesn't have much of a developer community, so there's no committed roadmap. That roadmap page is a collection of enhancements I wanted to add as well as others. If you want a feature, you will have to add it yourself. Or rally someone else to do it. :>) Your definition of aggregate is slightly different. Sum, length are column functions but group by is a collection operation. I think the query by column function is more doable than group by. You will need to write out the full function expression rather than the column alias in the SQL builder. Think you will need to parse that from the resource SQL. That will now be a bit trickier since commas are allowed in the functions. They could be in aliases too I suppose. You could ask for the expression as explicit XML metadata, slightly more work for the user. You will probably need to extend the column metadata class with an expression attribute, which is used during SQL building. On your first change for negative limit, does the offset stay positive? Let's say there are 100 rows and the page size you want is 50. So limit -50 offset 0 gives you the first set and offset 50 for the second page, all ordered descending? According to MySQL 5.7 select doc (http://dev.mysql.com/doc/refman/5.7/en/select.html) limit requires a non-negative integer. Could you provide more detail please? Second, please resubmit just the Java files with only the relevant lines changed. The metadata class has every line changed. Thanks! Mark |
|
Hi Mark, |
|
Oh okay. It's not a MySQL feature. It's a workaround for the missing order by support. Current ordering is always by the PK ascending. I think we should consider instead adding first class support for order by to the framework. The http interface could be something like: The limit/offset behavior is optionally applied. |
|
Yeah, that would be nice and more complete. |
|
It's a good addition. But it will still be quite a bit of effort for me to add test cases, execute across both supported DBs, try it in one app server, and update the documentation. You're welcome to proceed with that work. Hard for me to get behind a hack that isn't addressing the core feature gap. Mark |
Hi, as I'm using Netbeans, there are many auto-generated config files (I'm not sure if they are really needed, but...).
The actual change is in src/org/restsql/core/impl/AbstractSqlBuilder.java
when negative _LIMIT parameter is given, the sort order is changed to DESC, so we can query data at the end of the table (for example if we query for a max value of a column)