-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
Query interception support #334
Comments
@miere I believe this could be workable. Especially with the new extensions API, we are happy to support projects (such as a cache) that exist outside of core. If the caching layer becomes stable and popular enough we could then consider promoting it into the main build. I agree that targeting the Query API makes more sense -- SqlObject will turn into the query API behind the scenes, so if you target the core API you kill two birds with one stone. Maybe the first step is to take a look at the brand new extension API, and the query API, and determine exactly what hooks are necessary to allow the implementation? We could then have a PR adding the needed hooks, and you can develop your caching solution out of repo. Sound like a plan? |
@stevenschlansker |
@stevenschlansker I've removed my comment from the older issue. We can continue from here... ;)
So, do you think the best approach would be using the Fluent API? |
@miere Same as your original idea, except that instead of building on top of SqlObject methods, intercept at the lower fluent / query API level. Then anyone who uses either API can take advantage of your caching. |
Sorry for my late answer, @stevenschlansker
Could please send me an example of how to use these new extensions? I'm planning to write the first lines of code regarding to this issue soon... I hope you don't mind, but I would be happy to contribute... |
I also like the idea of a QueryInterceptor. Could the solution be implemented in a manner similar to StatementCustomizers where we update SQLStatement.internalExecute(QueryResultsMunger) to call registered QueryInterceptor's just after rewriting the sql but before creating the PreparedStatement? The QueryInterceptor interface would have to change to just receive a BaseStatement and/or a StatementContext or derived classes and then specify a return type that could be used to skip the PreparedStatement code. The return type is trickier as we still want the QueryResultsMunger to munge results. What do you think? Is there a better approach? |
@miere Take a look at the We are working on proper documentation, unfortunately progress is a little slow since all the core developers have day jobs :) but we're getting there! Until then the code is the only reference, sadly. There's a couple of simple plugins already |
Glad to hear you back, @stevenschlansker! Don't worry dude... I'll sure take a deep look into the JodaTimePlugin and GuavaPlugin to get some inspiration. I know how hard it is to create a community and contribute to OpenSource software having day jobs... On the next couple weeks I'll release the next kikaha's stable version, after that I'll be glad to help you guys with the query interceptor support. ;) |
Awesome, looking forward to it! |
Hi, @chscorpio, @stevenschlansker I'm back and ready to help! Before start to write the first lines of code regarding this issue I've spent some time to become familiar with the Jdbi3 API. At first glance: It is stable and look consistent than its previous version. Congrats! That said, I also took a deep look into the SqlStatement.internalExecute, how does it works and how the StatementCustomizers also works. After reading the code Long story short:
TL; DR;I don't know what you guys think about but, before your suggestion, I had something like the Filter implementation from Servlet API for the Jdbi. I was wondering to create a class where I could be notified of any query execution. After taking a deep look into the SqlStatement class I could realise what you guy meant with "I agree that targeting the Query API makes more sense", once everything really becomes a JDBC Statement. Considering this, I think this method would hold two arguments:
Also I was considering:
I'm using JDBI today in a project that handle thousands of thousands of queries per second to store some statistical data. At that project there is a few queries that compute some aggregations and these aggregations would take several minutes to conclude. I was wondering to cache only these queries, once the other one took less than 10ms. If I was able to Annotate a method from my SQL Object I would be able to identify which methods I would cache (or not) through the interceptor. Please note this was my first though, I'm not trying to push my code design as the definitive solution here. It's quite the opposite actually: I just want to clearly understand what you guys have exactly in your mind to ensure my contribution will be useful to Jdbi at all, not only my personal needs (cache stuffs). Cheers! \o |
Hi Miere, I've given this some thought too in the intervening months. I'm actually As part of the work on making It's not public API at the moment, but it could be made public with a For caching, you could define a decorating annotation e.g. @Cache (possibly I'm thinking that a guava-specific cache implementation would be a good Let's open an issue on Github and we can discuss it further there. |
You are talking about it on an issue on GitHub already ;) Thanks for diving so deeply in to understand! The reason I was pushing towards implementing this via the lower level statement API is that SqlObject still cannot cover 100% of use cases (nor is it really intended to) and there are still places where the fluent api is cleaner -- if the caching artifact is built on top of SqlObject then there is a disconnect about what is potentially cacheable. I think your idea of a Filter-like API with a context and statement handed to it could make sense. What does your |
Heh. I thought I was responding to the mailing list.. cone of shame |
Hehehehehe... @stevenschlansker I've forget to consider StatementContext. If we would have access to meta-data found at the SQL Object, we can use it. But, it took a deep look into the @qualidafial suggestion:
I was able to create a (draft of a) distributed cache here using this approach. It was very simple indeed. I see that the #411 came two months latter than this issue and it shows me how intense has been the work on Jdbi3 since then. So, think now that it is up to you guys: both approach is quite good to me. The QueryAPI approach has the advantage to be a central point of maintenance but the @qualidafial suggestion seems more cleaner. Also, if the |
I think the other thing is that if you're talking to the fluent API, you can just imperatively talk to a cache too. So in my mind, it makes more sense to focus on the SqlObject layer, and try to come up with a set of declarative annotations that cover the most common use cases, and try to structure it in a way that imperative (i.e. default or non-abstract) methods in a SqlObject have the same access to caches that declarative methods do. |
Opened #455 as a first step to enabling this. |
Hi guys, I'm back! Sorry I leave this issue alone for a while...
The original intention behind this issue was provide a way to intercept queries - or SQL Objects' methods. Since the PR #455 was merged I was able to create a decorator and intercept methods. It worked perfectly on both JDBI3 versions (alpha1 and alpha2).
I do think the same way, @qualidafial. I've spent a little time today creating a very tiny layer for Query Caching using Annotations, Decorators and SQL Objects... I didn't opened a PR for this yet, I just created the code on a sample project I have on the GitHub. I would like you guys take a deep look into the source code I've made and send me a (honest) feedback about it. The Cache Support source code could be found here. To sum up, this package contains a Decorator that looks for Here you can see an Integration Test with the caching mechanism in action. The query Please, send me feedback... okay? If you think this would be useful to JDBI3 I'll a PR tomorrow with this core implementation. I would also create a |
Hi Miere, this looks like a good start! I'm currently advocating for a change in core that will probably alter v3's I'm hesitant to adopt First off, this needs time to mature, and prove itself to work well for a variety of users and environments. If we did add it, I would probably keep the guava cache implementation separate from the Secondly, introducing caching is a bit of a philosophical departure (as I see it) for the project. JDBI has always acted like an interpreter between the java runtime and the database--merely translating commands and requests from one format into another without changing any semantics. Adding caching would make JDBI act as an agent (as Hibernate does) rather than an interpreter. I personally view the session cache as Hibernate's greatest weakness--once you open this door, it becomes very easy for the library to start getting cute with you, trying to be smart on your behalf instead of just doing what you told it to. JDBI has so far avoided falling into this trap, and I'm hesitant to start treading near it now. Please consider rereading my list of questions from the original mailing list thread for specifics. If you can come up with an API that answers those questions in a cache-implementation-neutral way, that would go a long way toward easing my concerns. |
@miere Please take a look at the latest code in JDBI 3 now provides a configuration mechanism, and you can add your own arbitrary config data by implementing public class Caching implements JdbiConfig<Caching> {
public Caching() {}
private Caching(Caching that) {
this.cacheMechanism = that.cacheMechanism;
}
private CacheMechanism cacheMechanism;
public CacheMechanism getCacheMechanism() {
return cacheMechanism;
}
public void setCacheMechanism(CacheMechanism cacheMechanism) {
this.cacheMechanism = cacheMechanism;
}
// and any other universal caching configuration
public Caching createCopy() {
return new Caching(this);
}
} Then, in your cache method handler: @RequiredArgsConstructor
public class CachedMethodHandler implements Handler {
final Handler base;
@Override
public Object invoke(Object target,
Method method,
Object[] args,
HandleSupplier handle) throws Exception {
CacheMechanism cacheMechanism = handle.getConfig(Caching.class).getCacheMechanism();
final SQLObjectMethod sqlObjectMethod = new SQLObjectMethod(method, args);
Object foundData = cacheMechanism.loadDataFor(sqlObjectMethod);
if ( foundData == null ) {
foundData = base.invoke( target, method, args, config, handle );
cacheMechanism.storeDataFor( sqlObjectMethod, foundData );
}
return foundData;
}
} I'm planning to release another v3 alpha to Maven Central with the config stuff later today. |
Quite nice improvement. I'll forward to this tonight. About the Caching implementation of JdbiConfig. How I'm supposed to notify the Jdbi that there is a new JdbiConfig available? Should I register it on the Plugin configuration? Or should I provide it as SPI implementation of JdbiConfig interface? |
JDBI will instantiate your config object the first time you request it for that type. Jdbi jdbi = ...
jdbi.getConfig(Caching.class).setCacheMechanism(new LocalCacheMechanism()); |
I didn't get around to releasing last night--did it just a minute ago. It should be available in Central as version |
@miere Have you tried out your caching approach with a recent build, using the config stuff? We just released 3.0.0-alpha10 today. Beta is probably next. |
@qualidafial thanks for asking... Actually, I wasn't able to give a chance for alpha4 as you've mentioned before... I've been a lil' busy this last few months. I'll take a deep look into it this weekend and then I give you a feedback... Okay? Regards |
Heads up, we released beta this morning |
Hi @qualidafial I just adapted my former cache code to the 3.0.0-beta0 structure and it... worked greatly! |
This issue has been open for a while. The conversation has been wide-ranging, but I believe we've satisfied the core need at this point. I'm closing this ticket. If there's any further scope you want to explore, let's discuss it in a separate issue. Thanks all for the great discussion and feedback. |
PS: This is a copied and pasted text from an earlier discussion about supporting query interception at the #134 issue
Hi guys, I would like to suggest a different approach at this discussion and I'd like to hear your honest feedback about it...
I was reading this topic before came here. And, I agree with you guys that support cache layers are totally application dependent. But, I would like to set a point here: support cache layers would be hard to maintain in the long term, but allow developers to cache its queries isn't that hard.
Let's imagine that, instead of providing a cache abstraction, JDBI could provide a way to "intercept" queries. Bellow is my suggestion:
Of course, there is a huge room for improvement on my suggested design, but I just want to make a point: is it simple enough to be included on JDBI?
Regards
The text was updated successfully, but these errors were encountered: