Ticket #275 (new task)
Overhaul Criteria
| Reported by: | hans | Owned by: | gamr |
|---|---|---|---|
| Priority: | normal | Milestone: | 2.0 |
| Component: | Generator | Version: | devel |
| Severity: | normal | Keywords: | criteria |
| Cc: |
Description
This ticket is for the large-scale Propel 2.0 task of overhauling the Criteria system, to create an intuitive and more flexible framework.
Requirements for new Criteria:
- The logical expression API must be completely consistent & intuitive.
- We need abilty to support custom SQL expressions without a big headache.
- We need to be able to determine & apply unique table aliases at query composition time.
- We need to break out the JOIN, GROUP BY, etc. into a higher-level class, because a single Criteria object has a focus that is too narrow to address the concerns of things like JOIN (unique table aliasing, etc.).
- We also want to keep in mind the identifier quoting (#3), as Criteria design choices may affect that.
See wiki:Development/Criteria for a more in-depth look at new Criteria plans.
Attachments
Change History
comment:2 Changed 4 years ago by hans
I updated wiki:Development/Criteria to reflect this implementation (and give more examples, etc.).
comment:3 Changed 4 years ago by hans
Converted the peers to use new Criteria(2) in changeset:384.
comment:4 Changed 4 years ago by hans
Renamed Criteria2 -> Criteria in changeset:385
comment:5 Changed 4 years ago by hans
- Added phpdoc comments to classes
- Consolidated the aux/util classes into *Classes.php files, removing the criteria/ dir.
- Removed the generated addSelectColumns() method, since it is now inside the Query class
- Some minor method renaming in ColumnMap
- Added more fluent API to Query
- Renamed ColumnExpression->getQueryColumn() to createQueryColumn(), to capture the fact that a new instance is created every call.
- Refactored methods like getIgnoreCaseSql to take a QueryColumn parameter
- Changed from using "Concrete" in QueryColumn classnames to "Actual". Maybe not much better, but at least doesn't have specific OO meaning.
comment:6 Changed 4 years ago by hans
Added support for passing either a Query or Criteria to the basic doSelect*() methods in [398].
comment:7 Changed 4 years ago by hans
In changeset:402, I committed some changes inspired by Dominik's Criteria rewrite (see ZIP file attached to this ticket).
More specifically:
- I created a StatementBuilder interface which provides the buildSql() method.
- BasePeer/Query was refactored, moving the contents of !BasePeer::createSelectSql() to Query->buildSql()
- A new LiteralSql class replaces SqlExpr
- Values in the ColumnValueExpressions can now be any StatementBuilder; the default is to wrap non-StatementBuilder values in a new BindValueWrapper class.
wiki:Development/Criteria will be updated to reflect these changes.
Changed 4 years ago by hans
-
attachment
qi.zip
added
Adding Dominik's Criteria rewrite (also includes some Propel classes)
Changed 4 years ago by hans
-
attachment
Criteriastructure.png
added
Attaching Criteria structure image from Ants
Changed 4 years ago by hans
-
attachment
propel2.zip
added
Attaching Propel2 zip, containing new Criteria code from Ants
comment:8 Changed 4 years ago by hans
Adding email from Ants for reference:
Hi,
unfortunately I haven't had a lot of time to work on Criteria2 lately. Been increibly busy specifing the next generation core logical datastructure for all our systems, making our subcontractors understand the distributed messaging infrastructure we specified and generally trying to get our development infrastructure to the 21st century. I have some experimental code ready but it's undocumented, not integrated into propel yet and needs reworking after some insights while developing it. I don't know if I can spare significant amounts of time to work on Propel code in the near future. I can share my thoughts on a good Criteria API and infrastructure though.
I will skip over some trickier stuff, that would not be important for the first iteration. I'll start with a high level description of the concepts I used so we are (hopefully) on the same page.
Query is a command that returns a list of objects of specified type that match the querys predicate. Some additional attributes are amount of objects to fetch and ordering to fetch in. This means that each query has a base type or root type for context in which the predicates are interpreted. For example a query in english would look something like this:
fetch a list of Books whose "author" reference field has "name" field with
value 'Orwell'.
To specify predicates I think we should have full featured language specified as an abstract syntax tree. That is, we have composite expressions (boolean and, equality of two expressions, etc.) and simple terminal expressions that are either literal values or references to attribute values. Basically we would use the interpreter pattern to define constraints in the object domain that we can later transform into the SQL domain. Predicates being interpreted in a root type context means that all the attribute references specify a full path from the root type to a value. eg. the previous example would have a context of Book and a field access of Book->author->name. In my experimental implementation the example predicate looks like this:
ComparisonExpression Object
[a:protected] => ReferencePathDecorator Object
[reference:protected] => ReferenceField Object
[refEntity:protected] => SingleTableEntity Object
[table:protected] => Authors
[class:protected] => Author
[isNullable:protected] =>
[isMultiple:protected] =>
[fieldMap:protected] => Array
[author_id] => id
[name:protected] => author
[child:protected] => StringField Object
[columnName:protected] => name
[name:protected] => name
[b:protected] => StringLiteral Object
[value:protected] => Orwell
[operator:protected] => =
How to construct the predicate is orthogonal to the rest of the discussion. There is nothing stopping us from implementing several different ways of constructing the AST, from parsing a string to explicit construct commands. I like the fluent style API in which every expression type has a factory method for each (or most) of the operations that can be performed with it. Constructing the full reference paths for attributes is a bit trickier to get right. I used static methods on domain model classes to lazily construct a flyweight field access object. Simple fields return objects of type ValueField which are usable as expressions. References to other objects return ReferenceField objects, which have a magic method to get a field from referenced class and wrap it in ReferencePathDecorator. The decorator approach has some rather unfortunate consequences of losing the attribute path when delegating composite operation creation to delegatees. A better approach would be to use many instances of ValueFields and merge the reference path implementation into them. Anyway the running example using my proposed API looks like this:
<?php Book::author()->name()->equals('Orwell'); ?>
To compile the query into SQL we first build a list of all fields referenced via criteria in domain model space.
Then, using Entity classes (which represent domain model class -> SQL mappings) we transform the list and selected fields into a query tree which is composed of table references, joins, subselects and unions. (Selected fields can be added with $query->setFetchStrategy(Book::author(), !QueryStatement::FETCH_STRATEGY_EAGER) ). This is actually pretty simple when we have exactly one table per class. The difficult part of dealing with cases of mixed class table and concrete table inheritance with embedded value objects and split tables can be implemented later. The information is all there hidden from the public API.
After that the query tree should be split into separate compilable querytrees when we have multiple parallel toMany joins. And then the SQLBuilder can compile the query trees into SQL statements.
Got a bit hand wavy at the end because I ran out of time. I'll attach a UML diagram to visualize this a bit and the really crappy proof of concept code, so you can understand my ideas a bit better.
If you have any questions or comments I'd love to hear them. I'll try to answer any questions tomorrow.
I hope my ramblings were at least a bit understandable.
--- Ants Aasma
comment:9 Changed 4 years ago by gamr
- Owner changed from hans to gamr
- Status changed from assigned to new
I am now the lead for Criteria redevelopment, taking over the ownership of this ticket. Most likely Criteria2 will become its own independent project. Criteria2 will now also implement a plugin type system so we can do fulltext searching as well as support for gis data for databases that support it.
comment:10 Changed 4 years ago by hans
Yes, this is great. I look forward to the new Criteria. When you get a chance, it would be great to see some of the new API on the wiki. It may not be ready for that yet, though.
comment:11 Changed 3 years ago by anonymous
"We need to be able to determine & apply unique table aliases at query composition time."
Thats great, it would also be nice to change the used table at execution time. Because i've Three NestedSets and three aquivalent models. It would be nice if i can use only one model with three different tables.
I just checked in a rough implementation of "Plan 1" (see wiki:Development/Criteria) in changeset:381.
Not all features are implemented yet, but this is closer to being a complete system than the ideas sketched out on the wiki page.
Since the Peer classes will need to be modified (and doing so would break the existing Criteria system), here is a simple example of constructing a query that doesn't actually use any Peer classes:
Of course, once we modify the generator templates (which is trivial), the API will be much simpler. We could expect it to look something like this:
There would also be a !BookPeer::createCriteria() method (remember that in this model Criteria will also be used for UPDATE and DELETE statements) and we could probably also have the doSelect() method take a Criteria parameter, if the user didn't care about other query options:
... but it does seem that usually users would want to control the order of the results, so having doSelect() alwyas take a Query may be just fine.