TYPO3 Core

Question is whether we add support for this for the caller (best but needs change in Extbase typically) or if we add this as a global setting in localconf.php (or in fact default_config.php).

This later would allow Extbase to be kept as this while being less fine-grained for extensions that currently use sql_query. But as they should not anyway and should only rely on exec_SELECTquery and other similar methods, I take it as an option too.

Ah, I understand. I think at least with the issue we rise awareness of the problem within the Extbase team. The idea of a global switch (or a setting in DBAL's ext_conf_template.txt) is fine.

What I don't understand is why sql_exec() should be more difficult to parse than when going to exec_SELECTquery etc. After all, once you have split up the query in these parts:

SELECT
FROM
WHERE
ORDER BY
GROUP BY
LIMIT

The parsing of the individual parts is the same as required in exec_SELECTquery, isn't it?

We are aware of this problem. I will implement the switch in Extbase as soon as it is implemented in DBAL.

Hi Ernesto,

The query is not more difficult when going in sql_exec(), this is why I made it available but the problem is that our SQL parser is fine as long as the query is kept "simple" and by simple I mean really not complicated and you have to be aware of limitations too, such as the inability for it to parse useless parentheses:

SELECT * FROM pages WHERE ((pid = 0)) || something-else

won't work!

(bug report exists for this btw)

Yes, agreed. But then the project to make DBAL work with Extbase would require some "lots" of more work and not only getting rid of sql_exec(). Because the "complex part to parse" probably are the WHERE and HAVING sections.

So +1 for the generic "pass-through sql_exec()" switch (in TYPO3_CONF_EXEC for example) so that installations with DBAL trouble could make use of DBAL (for regular standard queries) and still get extbase extensions to work. That won't even require a change in extbase (or other affected extensions).

Could you provide that patch for beta2 already? That would allow people to test the combination. Thanks!!

I'll implement that as a global switch in DBAL extension configuration (ext_conf_template.txt). It will be "on" by default as sql_query is badly supported anyway.

I'll commit a change to Core too to mark this method as "dangerous" when using DBAL, this way, developers know this should really be avoided!

Please report here if patch works for you (and whether Core patch is OK for you). I'll then commit it as FYI for Core and as such to DBAL trunk.

Core patch is ok.

I cannot test if the pass-through helps, because I don't have an use-case where it fails. Maybe some extbase guy could pre-test it?

Anyway, I would just go ahead and add it, and we can get people to test it in beta2: we'll make a big note about it in the release notes.

Thanks!

Cheers,
Ernesto

+1 by reading and testing

But I was not able to break DBAL with an Extbase extension either.

The task of writing a universal parser for any given SQL in another dialect is huge. So Kudos to Xavier.

Sidenote: IMO Extbase is using the core API correctly regarding the comment above sql_query() and the code of the method. The fact that DBALs overrides this method and changes its behavior is out of reach of Extbase. The switch is a good intermediate solution.

- Committed core patch to trunk as rev. 9688
- Committed DBAL patch to trunk as rev. 40681