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

Proposition for a new SQLHelper #311

Closed
marcioAlmada opened this issue Nov 7, 2013 · 33 comments
Closed

Proposition for a new SQLHelper #311

marcioAlmada opened this issue Nov 7, 2013 · 33 comments

Comments

@marcioAlmada
Copy link
Contributor

Intent

This issue intends to discuss a PROPOSITION to add a new SQLHelper to redbean: I was looking at this post on redbean forum and noticed a promising idea from @daviddeutsch called Rx:

R::$x->project->name('project x')->code(200)->find();

Usage

Using SQL operators (=, >, <, <>, LIKE, BETWEEN, IN, etc):

$R::
    $x->project
        ->name('like', '%project x')
        ->priority('>', 5)
        ->created_at('between', [$time1, $time2])
        ->find();

And possibility to group SQL conditions:

R::$x->project
    ->name('like', '%secret%')->AND->priority('>', 9)
    ->OR
    ->OPEN
        ->code('in', [007, 51])->AND->created_at('between', [$time1, $time2])
    ->CLOSE
    ->find()

Sane defaults:

Default SQL operator is = and default SQL construct between conditions is AND, so Rx original syntax would still be allowed :)

R::$x->project->name('project x')->code(200)->find();

I know redbean philosophy about query writers, but this does not looks like over engineering IMMO and would be a great addition. Maybe I could implement this feature, but need some feedback to improve the idea.

@gabordemooij What do you think?

cheers!

@daviddeutsch
Copy link
Contributor

We're kinda-sorta discussing just that over at #309 😉 Also - some of what you're talking about there is already working in https://github.com/daviddeutsch/rx

@marcioAlmada
Copy link
Contributor Author

This is great! does it already supports SQL operators?

@daviddeutsch
Copy link
Contributor

In general, the problem is that the fluid syntax kind of limits the number of methods you could use. Then again, if we're just talking about stuff like ->age('in, [17]), then that is something that could definitely be considered. Right now, a method call translates into a property with a single parameter being the property value. Making it "if it's two values, do something clever" shouldn't be hard.

@zewa666
Copy link
Contributor

zewa666 commented Nov 7, 2013

We also had the discussion regarding LINQ and RedBean already.
The way Linq handles data manipulation is by operating with similar mappers/reducers/aggregators to achieve a different result from a previous enumerable. Maybe we should take a closer look at some of those multiple Linq2PHP ports to get some nice ideas about what we could include as well.

@daviddeutsch
Copy link
Contributor

Yeah, that's a good point - we should definitely look at some of the existing solutions so that we can draw a map of what kind of special tools we want to build. I'm kind of visualizing a Venn Diagram here - where we have maybe 5-7 tools that each go in a certain direction and you would use whatever fits your current problem best.

@marcioAlmada
Copy link
Contributor Author

I'm trying to keep this idea because it looks far more natural than other query builders, despite it's limitations. How about:

$R::
    $x->thing
        ->name('like', '%stuff')
        ->AND
        ->look('gross')
        ->AND
        ->age('in', [17])
        ->AND
        ->created_at('between', [$time1, $time2]);

@gabordemooij
Copy link
Owner

Looks good, but bit confused here, are we aiming for a new query writer or a new finder syntax? or a combination?
also see my comment in #309

@marcioAlmada
Copy link
Contributor Author

I don't have any knowledge about Redbean internals so I'm not sure what's the difference between finder and query writer. Are "query writers" the classes Redbean uses to write SQL for each supported database? In positive case, maybe I'm using the wrong terms.

I'm trying to extend a little more of @daviddeutsch awesome Rx idea and reduce it's use case limitations so people could fall back less frequently to manual queries (or hybrid APIs mixing method calls and SQL fragments). I have a felling that 80% of the time people are retrieving beans applying just some simple groups of conditions, limits and offsets.

Yes, I guess it kinda blurry the lines between a query writer and a finder. Do you guys see any problem with that?

@daviddeutsch
Copy link
Contributor

@marcioAlmada I think exactly that line is what we're trying to find here. Maybe R::$x is best left simple - the $x is supposed to stand for "express" anyhow. What you're going at is a little more complex, so maybe we would have another find helper R::$c which allows for stuff like nesting.

Mostly the trouble I'm having thinking about your proposal is coming up with good use cases. Since R::$x is already using "AND" concatenation, an R::$c would be about using "OR", or mixing "AND" and "OR". But I find that a lot of the time, it's more of a question of how you set up your data structure. Complex SQL queries can be a sign of not having put enough thought into your setup to begin with.

What might make sense in R::$x is a mode-switch between "AND" and "OR". So maybe something like:

// Use AND
R::$x->thing
    ->name('like', '%stuff')
    ->look('gross')
    ->age('in', [17])
    ->created_at('between', [$time1, $time2])
    ->find();

// Use OR
R::$x->thing
    ->name('like', '%stuff')
    ->look('gross')
    ->age('in', [17])
    ->created_at('between', [$time1, $time2])
    ->looseFind();

As for your proposed syntactic sugar with the ->AND calls, I think you would go with something like this instead. I have constructed a hypothetical scenario where you would split up the four AND statements into two times two AND statements that are OR concatenated:

R::$x->thing
    ->maybe( R::$x->name('like', '%stuff')->look('gross') )
    ->maybe( R::$x->age('in', [17])->created_at('between', [$time1, $time2]) )
    ->looseFind();

And when it iterates over queries in Rx_FindHelper::makeQuery(), it would resolve the FindHelper to an SQL string. ->maybe() would constitute it's own sub unit of the query and within that sub query, we construct a blank call of Rx_FindHelper that gets resolved within ::makeQuery() (note that they lack ->find()).

An inverted version would look like so:

R::$x->thing
    ->maybe( R::$x->name('like', '%stuff')->look('gross')->loose() )
    ->maybe( R::$x->age('in', [17])->created_at('between', [$time1, $time2])->loose() )
    ->find();

Hmm, that kind of does away with the idea of "this stuff isn't suitable for R::$x and we might need R::$c" now, doesn't it? 😉

@marcioAlmada
Copy link
Contributor Author

First approach:

  • BAD: it totally fails with the scenario you created, leading people to go for hybrid sql fragment + method calls we already addressed

Second approach:

  • GOOD: offers a good way to combine conditions
  • BAD: R::$x repetition in userland code

Same scenario with proposed syntax:

R::$x->thing
    ->name('like', '%stuff')->AND->look('gross')
    ->OR
    ->OPEN
        ->age('in', [17])->AND->created_at('between', [$time1, $time2])
    ->CLOSE
    ->find()

@daviddeutsch
Copy link
Contributor

@marcioAlmada First approach: Not sure how it fails - maybe you could elaborate. I only added the SQL stuff because I was copying from your earlier example.

Second approach: I have no idea what "R::$x repition in userland code" means.

As for the proposed syntax: Look, turn that idea clock of yours a couple more turns forward and basically what you end up with is SQL. The basic drive that we have right now is trying to find sharp tools that do a small number of jobs. If you keep trying to make them do everything then that breaks down pretty quickly.

So the question that you want to answer is: What is my use case?

Not: How can I cover more use cases?

@marcioAlmada
Copy link
Contributor Author

First approach fails to satisfy the scenario you explained:

I have constructed a hypothetical scenario where you would split up the four AND statements into two times two AND statements that are OR concatenated:

About userland code repetition: I meant that a given user would need to access R::$x several times to compose the group of conditions. This compared to fluid syntax seems inferior IMMO. Maybe codebase can be easier to maintan, but userland code starts to gets cluttered.

R::$x->thing
    ->maybe( R::$x->name('like', '%stuff')->look('gross') )
    ->maybe( R::$x->age('in', [17])->created_at('between', [$time1, $time2]) )
    ->looseFind()

Just to clarify, when I started this issue I had no idea Rx already existed, so I'm not criticizing Rx. I just intend to propose a finder that can satisfy more use cases without loosing too much of Rx express factor :)

I'm trying to extend a little more of @daviddeutsch awesome Rx idea and reduce it's use case limitations so people could fall back less frequently to manual queries (or hybrid APIs mixing method calls and SQL fragments). I have a felling that 80% of the time people are retrieving beans applying just some simple groups of conditions, limits and offsets.

@daviddeutsch
Copy link
Contributor

No worries, I did not take offense or anything, just trying to figure out where you're going with this.

I liked my nested approach because it reduces the amount of thinking overhead that people have to do. A problem that just occurred to me though is that we cannot have that kind of nesting anyhow because we're using a static class with R::$x. So that kills that idea right there.

@marcioAlmada
Copy link
Contributor Author

Just to keep discussion flowing, here is how I imagine finder right now:

  • should support sql operators, default is =
# using default "=" operator
R::$x->thing->name('stuff')->find();

# using another operator instead of default one
R::$x->thing->name('like', '%stuff')->find();
  • should support condition concatenations (AND, OR, etc)
R::$x->thing
    ->name('like', '%stuff')
    ->AND
    ->look('gross')
    ->AND
    ->age('in', [17])
    ->AND
    ->created_at('between', [$time1, $time2])->find();
  • should support grouped conditions
R::$x->thing
    ->name('like', '%stuff')->AND->look('gross')
    ->OR
    ->OPEN
        ->age('in', [17])->AND->created_at('between', [$time1, $time2])
    ->CLOSE
    ->find();

@gabordemooij
Copy link
Owner

I like the syntax @marcioAlmada I think adding OPEN(), CLOSE() and array-to-comma-separated-list features would be nice.

However we are now creating a real query builder here and there are some issues with that:

  • People have to learn a new syntax, why? they already know SQL
  • By translating statements like created_at and look into SQL we become responsible for making this work across all database platforms, unless you have a plan to translate all these constructs to Oracle I fear this is not a very useful idea

The idea behind SQLHelper was never to build a Query Builder. Rather, I tried to find a way to share SQL statements without having to pass strings.

@marcioAlmada
Copy link
Contributor Author

@gabordemooij Oops. I should have clarified that all method calls translate to database fields, so they are not specific functions from finder :) Sorry for the bad examples.

Take a look at this:

R::$x->beantype->property('<value>')->OR->property('<sql operator>', 'value') [...] ->find();

The only real constructs from finder would be AND, OR, OPEN, CLOSE. I guess they should behave equally with all databases.

@marcioAlmada
Copy link
Contributor Author

@gabordemooij Most of the time I'm using Postgres and Oracle, so I would not build a MySQL only plugin... also, I'm more motivated to implement this issue before #313.

This leaves one question: How can plugin developers write database agnostic tools on top of Redbean API?

@gabordemooij
Copy link
Owner

By using methods from the QueryWriter.
If you need more than that I recommend to extend the API with your own QueryWriters.
Is that what you mean?

@marcioAlmada
Copy link
Contributor Author

OK. Probably query writers already have everything needed. Let me try to implement this ;)

@gabordemooij
Copy link
Owner

Cool!

@marcioAlmada
Copy link
Contributor Author

HI again. Found some troubles here. I expected QueryWriter had convenient methods like:

$writer = R::$toolbox->getWriter();
$writer->createLikeCondition('field', '%some string%');

And it would return an SQL snippet compatible with the current DB user is using:

// > "field LIKE '%some string%'"

But it seems QueryWriters are classes with very high level operations like createField() 😄 So I suppose it's not QueryWriter what I should be using. Are there any family of classes that aim to create small parts of SQL for each supported database?

@gabordemooij
Copy link
Owner

The QueryWriters are very high level.
The idea behind this approach was to make as little methods as possible so it's quite trivial to support another database. If I would have added methods to support all of the basic SQL functions the writers would have grown big and it would take a lot of time to support another database.

@marcioAlmada
Copy link
Contributor Author

I'll end up creating a quite huge infrastructure for a plugin, but already know hot to achieve it. Thanks.

@gabordemooij
Copy link
Owner

Before you start... I hope you don't mind I won't include this in the core. RedBeanPHP is meant to be a simple, compact ORM, this will be a plugin or module. I tagged the issue accordingly. I don't know what to do with the old SQLHelper though, I might just remove it all together.

@signalfrax
Copy link

I think the power of RedBeanPHP is in it's simplicity and we should keep it that way. The constructs above seems to deviate from that.
Usually when you need something like this : R::$x->thing
->name('like', '%stuff')
->AND
->look('gross')
->AND
->age('in', [17])
->AND
->created_at('between', [$time1, $time2])->find();
I think you are better of writing this in SQL using R::getAll();

@zewa666
Copy link
Contributor

zewa666 commented Nov 11, 2013

But if it is created as plugin the nice thing is that everybody can choose whether he/she likes it and add it if needed. That way we still have the functionality if desired and on the other hand a clean core.

@signalfrax
Copy link

A plugin can be a good idea as long as the core is not rewritten to fit this plugin.
And honestly I think we should encourage other developers to use simple solutions.
In my opinion by giving them this plugin you are encouraging the opposite.

@zewa666
Copy link
Contributor

zewa666 commented Nov 11, 2013

Well if you want perfomance , simplicity and no extra constructs to learn one could argue why to use an ORM at all. I think its a good idea to offer alternatives, whether you like them or not is ones personal opinion but second guessing what is right for everybody else is dangerous. keeping the core clean and unmodified OK. But Plugins are free to choose and also free to ignore

@marcioAlmada
Copy link
Contributor Author

@gabordemooij No problem. You're right, this should be a plugin. Maybe the SQLHelper should stay to keep backwards compatibility. I'll try to use SQLHelper so less things need to be implemented.

@zewa666 Exactly. Also it might take much much less than a millisecond of difference between a routine using getAll and a routine using a plugin like that.

@signalfrax It only depends of what you consider simple. The example you cited is only a fraction of what can be done with a helper. The other cases involve the $conditions problem (take a look at the use cases presented here #313), when dynamism is needed, a helper can be much more programmatic than writing SQL by hand and doing nasty string manipulations.

@gabordemooij
Copy link
Owner

@marcioAlmada Yes, you're right. Should keep it backward compatible. Some developers might rely on the old SQLHelper, would be quite annoying if I removed it. It's also quite tiny so does not really matter.

@daviddeutsch
Copy link
Contributor

Just pitching in here: I would definitely make this a new thing. Not just for the sake of backwards compatibility, but also to go with our new plan of keeping things sharp and modular.

@marcioAlmada
Copy link
Contributor Author

@daviddeutsch Fully agree. Also, SQLHelper is being used indirectly at some points so it is still necessary IMMO.

UPDATE:

@gabordemooij @zewa666 @daviddeutsch OK, finally redsql builds are passing (easier than expected):

  • postgre
  • mysql
  • sqlite
  • oracle

@marcioAlmada
Copy link
Contributor Author

RELEASE

So the plugin was baked months ago and has been used by a great amount of people without problems. Everything is 100% compatible with all redbean supported databases and version 1.0 will be released in a few days.

I took care to keep database support extensible by implementing filters. Some examples of filter implementations are Oracle LIMIT clause support (code) and the PostgreSQL ILIKE operator (code), but most filters are just ridiculous snippets like this one so you get the idea.

This under the hood architecture will appear fugly to some 😄 This will probably be reworked on 2.0. Plans are to create a simple SQL AST so SQL will be rendered only in the last moment. Filters will modify the AST not the SQL reference directly.

Along with composer, there is a phar release too for the ones that prefer the good and old single file releases: redsq.phar.

PS: needs some testing against redbeanKS

Cheers!

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

No branches or pull requests

5 participants