Feature #24145

Provide a pass-through flag for sql_exec() which goes directly to the native MySQL engine

Added by Ernesto Baschny almost 9 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Should have
Category:
Database API (Doctrine DBAL)
Target version:
-
Start date:
2010-11-20
Due date:
% Done:

0%

PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

DBAL currently tries to parse queries passed to it via sql_exec(). This is marked as "experimental" in 4.4 but brings lots of troubles on complex queries, for example the queries generated by Extbase.

The idea of parsing the queries is that DBAL also has to handle "mappings" for tables.

A new feature proposed by Xavier would enable the caller to "pass-through" the sql_exec() directly to the underlying MySQL native driver, meaning no parsing at all in DBAL. This would allow running Extbase only on MySQL but at least work even if DBAL is active.

This is an intermediate fix which should be added to 4.5 to rise the usage of both DBAL and Extbase at the same time. Keep in mind that with this change, Extbase will work only on MySQL (as it currently does anyway).

In future release of TYPO3, this problem might be solved already, as soon as their is either a query-generation API in DBAL and/or some enhancements on the Extbase side of query generation.

(issue imported from #M16491)

16491_core.diff View (521 Bytes) Administrator Admin, 2010-11-23 11:34

16491_dbal.diff View (1.35 KB) Administrator Admin, 2010-11-23 11:34

History

#1 Updated by Xavier Perseguers almost 9 years ago

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.

#2 Updated by Ernesto Baschny almost 9 years ago

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?

#3 Updated by Jochen Rau almost 9 years ago

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

#4 Updated by Xavier Perseguers almost 9 years ago

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)

#5 Updated by Ernesto Baschny almost 9 years ago

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!!

#6 Updated by Xavier Perseguers almost 9 years ago

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!

#7 Updated by Xavier Perseguers almost 9 years ago

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.

#8 Updated by Ernesto Baschny almost 9 years ago

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

#9 Updated by Jochen Rau almost 9 years ago

+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.

#10 Updated by Xavier Perseguers almost 9 years ago

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

#11 Updated by Susanne Moog over 8 years ago

  • Target version deleted (4.5.0)

Also available in: Atom PDF