qom factory cleanup

6 views
Skip to first unread message

David Buchmann

unread,
Jul 25, 2013, 3:41:35 AM7/25/13
to jackal...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

hi,

i went to fix the selectorName issue we have between jcr and phcpr.
this turned out to be bigger than expected in terms of changes in
phpcr-utils and jackalope.

i would be happy if people could review the PRs

https://github.com/phpcr/phpcr/pull/68
https://github.com/phpcr/phpcr-utils/pull/86
https://github.com/phpcr/phpcr-api-tests/pull/115
https://github.com/jackalope/jackalope/pull/185

also, i did not change anything on the query builder. not sure if that
is fine like this, or if there are missing tests and i am creating
problems.
if somebody could test those 4 PR with phpcr-odm and the cmf sandbox
and report problems, that would be great. or also with your custom
projects if you use queries, the qom factory or the phpcr-utils query
builder

cheers,david
- --
Liip AG // Agile Web Development // T +41 26 422 25 11
CH-1700 Fribourg // PGP 0xA581808B // www.liip.ch
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJR8NarAAoJED/JtliXIA4sXeoH/ja+f/agq+GfczWaM3bscjgU
C8Gn02rFd8HTSTOynlai0xZmtY0dSgxWJPi9yGSmeGndbGmYeg3KTkWI/BQk9qjT
So/dTKIJudsypEiYu+lunCmTFYuFepO8SGNT0KFYfDsPPi+5sLCRzOl9vG4oLRE4
uwwMvuARa2lnAmWPZa89WXEZhbpXsZL16xWTcwiXeaa556fJE7Ldp59j8DpzvO/q
rzggf1R0c5Y3QLQIYwhoKQs24He42TpQvVMl8022BuIRCbTPvU6AmcxsFhYb3VLj
4qwilYyhcZqPsjv7fzCJS+OX5ImeFXs+unI8nnQRdzMysh9iP0gPVCJtCKJHH/0=
=omfy
-----END PGP SIGNATURE-----

Lukas Kahwe Smith

unread,
Jul 25, 2013, 6:45:58 AM7/25/13
to David Buchmann, jackal...@googlegroups.com

On Jul 25, 2013, at 9:41 , David Buchmann <david.b...@liip.ch> wrote:

> Signed PGP part
> hi,
>
> i went to fix the selectorName issue we have between jcr and phcpr.
> this turned out to be bigger than expected in terms of changes in
> phpcr-utils and jackalope.
>
> i would be happy if people could review the PRs
>
> https://github.com/phpcr/phpcr/pull/68
> https://github.com/phpcr/phpcr-utils/pull/86
> https://github.com/phpcr/phpcr-api-tests/pull/115
> https://github.com/jackalope/jackalope/pull/185
>
> also, i did not change anything on the query builder. not sure if that
> is fine like this, or if there are missing tests and i am creating
> problems.
> if somebody could test those 4 PR with phpcr-odm and the cmf sandbox
> and report problems, that would be great. or also with your custom
> projects if you use queries, the qom factory or the phpcr-utils query
> builder

Jackalope tests run through nicely for Jackrabbit

There are some issues with Doctrine DBAL however. Actually it raised a question on the Jackalope changes:
https://github.com/jackalope/jackalope/pull/185#pullrequestreviewcomment-5394625

Also it once again illustrates that the QOMWalker cannot embedd any nodetype or selector names into the SQL.

Right now we have the following method call to get the SQL:
$sql = $qomWalker->walkQOMQuery($query);

I think this method should also return an alias (as used within the SQL) to selector mapping. This would mean we can drop the selector code from the Client::query() method which seems a bit redundant given that we already traverse the selectors on the QOMWalker.

It would however mean either returning an array or defining a new class from which to fetch the SQL, the selectors (and possibly the main selector).

For PHPCR ODM we need to update the QueryBuilder, I guess we will now have to require that the from()/nodeType() is called first.
I will take care of that today and then check the cmf-sandbox.

regards,
Lukas Kahwe Smith
sm...@pooteeweet.org



signature.asc

David Buchmann

unread,
Jul 25, 2013, 7:13:41 AM7/25/13
to Lukas Kahwe Smith, jackal...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

thanks a lot for the feedback lukas.

yeah, i fear that there is quite some missing tests on the
querybuilder so we did not see the issues.

> For PHPCR ODM we need to update the QueryBuilder, I guess we will
> now have to require that the from()/nodeType() is called first. I
> will take care of that today and then check the cmf-sandbox.

in the sql2 parser i just made the column selection not create qom
objects but delay until the FROM part is parsed. maybe we could do the
same here. otherwise i think the restriction on the order would be ok
for me too.

cheers,david
- --
Liip AG // Agile Web Development // T +41 26 422 25 11
CH-1700 Fribourg // PGP 0xA581808B // www.liip.ch
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJR8QhiAAoJED/JtliXIA4sfNYH/iXxIyiw97y2ryrqaXR0qgac
ZsZgwWvgCXDIQ4tPqkkGv5lgd8yVuW64QT5GovgcFA3tCESdTTnJbUofCigEvIWT
hTI8dsbVKZATSOT4ViRhsRuV2oLBqciQt8J3AbLXoDNSlESJ5l3Sk59rPOQJmVLL
gtkL9bCCAPNuVERGkwwS1x1RVKRiZ4ttc6vt0C8pgI1IMYsyZ5G0z8uq0H83QLP+
tmkOFhLdTtf9YScutt80ZgkDlWFmIGwcR45/ZUg4QpySF/dxzPL9r1LxZVfJhGR/
bw5QIZAr4b1KP6zm4UduJlJU+pAphw++WEUGoJ2IQAr2Y6POxmyjBE1XF02C5l0=
=3ZYd
-----END PGP SIGNATURE-----

Lukas Kahwe Smith

unread,
Jul 25, 2013, 12:17:10 PM7/25/13
to David Buchmann, jackal...@googlegroups.com
https://github.com/jackalope/jackalope-doctrine-dbal/pull/140

> For PHPCR ODM we need to update the QueryBuilder, I guess we will now have to require that the from()/nodeType() is called first.
> I will take care of that today and then check the cmf-sandbox.

i will probably work on this on the weekend.
signature.asc
Reply all
Reply to author
Forward
0 new messages