jackalope / jackalope-doctrine-dbal

Doctrine DBAL transport implementation for Jackalope
http://jackalope.github.io
Other
144 stars 60 forks source link

[query] Selector does not allow specification of mixin #217

Open dantleech opened 10 years ago

dantleech commented 10 years ago

The doctrine dbal selector does not seem to support selecting from mixin types, for example sulu:content is a mixin type:

PHPCRSH > SELECT * FROM [sulu:content] AS c WHERE ISCHILDNODE('/cmf/sulu_io/contents/test') AND c.[i18n:de-state] = 2;
+------+
| Path |
+------+
PHPCRSH > SELECT [jcr:mixinTypes] FROM [nt:unstructured] AS c WHERE ISCHILDNODE('/cmf/sulu_io/contents/test') AND c.[i18n:de-state] = 2;
+-----------------------------------+------------------+
| Path                              | c.jcr:mixinTypes |
+-----------------------------------+------------------+
| /cmf/sulu_io/contents/test/foobar | [0] sulu:content |
| /cmf/sulu_io/contents/test/barfoo | [0] sulu:content |
+-----------------------------------+------------------+

http://www.day.com/specs/jcr/2.0/6_Query.html#6.7.3 Selector:

the node’s primary node type is nodeType, or the node’s primary node type is a subtype of nodeType, or the node has a mixin node type that is nodeType, or the node has a mixin node type that is a subtype of nodeType

Jackrabbit supports mixin as selector.

dantleech commented 10 years ago

I guess we would need to additionally have a field for mixinTypes in the schema:

MariaDB [sulu]> explain phpcr_nodes;
+----------------+--------------+------+-----+---------+----------------+
| Field          | Type         | Null | Key | Default | Extra          |
+----------------+--------------+------+-----+---------+----------------+
| id             | int(11)      | NO   | PRI | NULL    | auto_increment |
| path           | varchar(255) | NO   | MUL | NULL    |                |
| parent         | varchar(255) | NO   | MUL | NULL    |                |
| local_name     | varchar(255) | NO   | MUL | NULL    |                |
| namespace      | varchar(255) | NO   |     | NULL    |                |
| workspace_name | varchar(255) | NO   |     | NULL    |                |
| identifier     | varchar(255) | NO   | MUL | NULL    |                |
| type           | varchar(255) | NO   | MUL | NULL    |                |
| props          | longtext     | NO   |     | NULL    |                |
| depth          | int(11)      | NO   |     | NULL    |                |
| sort_order     | int(11)      | YES  |     | NULL    |                |
+----------------+--------------+------+-----+---------+----------------+

and add it to the node type clause method: https://github.com/jackalope/jackalope-doctrine-dbal/blob/master/src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php#L900

lsmith77 commented 10 years ago

we currently only store the mixin inside the props XML data?

now if we extract this from the XML data, then we will have to deal with the fact that it can in fact be multiple mixins and of course there is also inheritance there. Maybe it might make sense to have a separate table with the mapping of the mixins. We then determine if the selector is a node type or a mixin. If it is a mixin, we join that additional table. This would also make it easier to not require schema changes for current running code.

dantleech commented 10 years ago

Would it make sense to have both primary node types and mixins in that table so we handle both things in the same place

lsmith77 commented 10 years ago

if I specify a node type in the FROM, then the current DB schema works quite well and is very performant.

Now in the case when a mixin is specified we would need an additional join per selector with a mixin. In this table we would store the mixin's assigned explicitly to the node. Inheritance is likely better handled by generating a list of possible mixin's inside the query.

In this sense I do not see any benefit if also having the node type in that table.