Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Subquery in query builder #5810

Closed
SupremeSimon opened this issue Mar 18, 2022 · 5 comments
Closed

Bug: Subquery in query builder #5810

SupremeSimon opened this issue Mar 18, 2022 · 5 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer duplicate Issue or pull request duplicates an already existing issue/pull request

Comments

@SupremeSimon
Copy link

SupremeSimon commented Mar 18, 2022

PHP Version

8.0

CodeIgniter4 Version

4.1.9

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MySQL 8.0.27

What happened?

The CI query breaks with a written subquery that contains a WHERE AND in it.

I have a query that needs to build from a base query and data from a datatable.
When the users search on the datatable, the search value gets passed into a function that adds the search value into a orLike query builder function.

In debugging I've just written this query manually and the same issue occurs.

I've only noticed it break when you add an AND to it. A single WHERE it doesn't have a problem with.

I have also found out that if you switch off the escape function, the query generates correctly. (Although in test this morning with the most basic query example I'm going to use in the reproduce step, it's actually not fixing it)

Steps to Reproduce

The full query I'm using is:

        $builder->select("`c`.`contact_id`, `c`.`file_as`,");
        $builder->select("(SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1)");
        $builder->join("correspondence corr", "corr.contact_fk = c.contact_id", 'LEFT');
        $builder->join("contact_person cp", "c.contact_id = cp.contact_id", 'LEFT');
        $builder->join("address_contact_link acl", "c.contact_id = acl.contact_fk", 'LEFT');
        $builder->join("address ad", "ad.address_id = acl.address_fk", 'LEFT');
        $builder->join("contact_type_link ctl", "c.contact_id = ctl.contact_fk", 'LEFT');
        $builder->where('c.agent_fk', session("agent_id"));
        $builder->where('c.contact_class', 'person');
        $builder->orLike("(SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1)", "", "both", false);
        $builder->groupBy("c.contact_id");

        echo $builder->getCompiledSelect();

Without the and in the orLike function or with the escape set to false I get a working query:

        SELECT `c`.`contact_id`, `c`.`file_as`, (SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1) FROM `contact` `c` LEFT JOIN `correspondence` `corr` ON `corr`.`contact_fk` = `c`.`contact_id` LEFT JOIN `contact_person` `cp` ON `c`.`contact_id` = `cp`.`contact_id` LEFT JOIN `address_contact_link` `acl` ON `c`.`contact_id` = `acl`.`contact_fk` LEFT JOIN `address` `ad` ON `ad`.`address_id` = `acl`.`address_fk` LEFT JOIN `contact_type_link` `ctl` ON `c`.`contact_id` = `ctl`.`contact_fk` WHERE `c`.`agent_fk` = '1' AND `c`.`contact_class` = 'person' OR (SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1) LIKE %% GROUP BY `c`.`contact_id`

If you set the escape to true and have the AND 1 = 1 in there, you get the following. (I'm using AND 1 = 1 as an example, There are many other AND's that I had in the actual query, I've just removed them for simplicity, the behavior is the same)

        SELECT `c`.`contact_id`, `c`.`file_as`, (SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1) FROM `contact` `c` LEFT JOIN `correspondence` `corr` ON `corr`.`contact_fk` = `c`.`contact_id` LEFT JOIN `contact_person` `cp` ON `c`.`contact_id` = `cp`.`contact_id` LEFT JOIN `address_contact_link` `acl` ON `c`.`contact_id` = `acl`.`contact_fk` LEFT JOIN `address` `ad` ON `ad`.`address_id` = `acl`.`address_fk` LEFT JOIN `contact_type_link` `ctl` ON `c`.`contact_id` = `ctl`.`contact_fk` WHERE `c`.`agent_fk` = '1' AND `c`.`contact_class` = 'person' OR (SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = `c`.`contact_id` AND 1 = 1) LIKE :(SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = `c`.`contact_id` AND 1 = 1): ESCAPE '!' GROUP BY `c`.`contact_id`

NOTE the ":" at the start and end of the LIKE statement and the lack of "%%". This query is broken now.

In tests done whist writing this. I broke down my query to its fundamentals. A sub query in a WHERE statement.

        $builder->select("(SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1)");
        $builder->WHERE("(SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1)", "test", "both", false);
        $builder->groupBy("c.contact_id");

        echo $builder->getCompiledSelect();

This query, regardless of if the escape is on or off, produces:

        SELECT (SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = c.contact_id AND 1 = 1) FROM `contact` `c` WHERE (SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = `c`.`contact_id` AND 1 = 1) :(SELECT COALESCE(GROUP_CONCAT(CONCAT(corr.description, ' ', corr.note) ORDER BY corr.order_field ASC SEPARATOR ' '), '') FROM correspondence corr WHERE corr.contact_fk = `c`.`contact_id` AND 1 = 1): GROUP BY `c`.`contact_id`

Again with the ":" at the start and end of the WHERE statement (In this case, just a WHERE, so this issues doesn't just relate to the orLike)
But removing the AND 1 = 1 or any additional WHERE statement will cause the query to generate correctly.

I hope this is enough info to reproduce.

Expected Output

I am expecting the query to generate correctly without the ":" characters getting added.

The query I'm writing should be valid even with it being a subquery in a where statement.

Anything else?

No response

@SupremeSimon SupremeSimon added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 18, 2022
@kenjis
Copy link
Member

kenjis commented Mar 19, 2022

Thank you for taking time.

First, I just want to ask you why do you use such a complex SQL statement with Query Builder?
If you use query(), you could write any SQL statement.
https://codeigniter4.github.io/CodeIgniter4/database/queries.html#regular-queries

Next, your sample code is too long to see and understand. Please write minimum code to reproduce it.

I think Query Builder does not work when you set such a complex SQL string in an argument and escaping on.

The next minor version v4.2.0, Query Builder will have enhancement of Subqueries.
See https://codeigniter4.github.io/CodeIgniter4/changelogs/v4.2.0.html#enhancements
You could try it if you use develop branch.

@MGatner MGatner added the database Issues or pull requests that affect the database layer label Mar 19, 2022
@SupremeSimon
Copy link
Author

SupremeSimon commented Mar 20, 2022

Hi,

I did suspect it was a lot.

First, I just want to ask you why do you use such a complex SQL statement with
Query Builder?

We are using datatables on our site, and are using dynamic columns on them, so if the column is included, is passes a data element or table column name to a class function that then builds the datatable results. The only way I can see of doing the correspondences column is with a subquery as a contact can have more than one tel number / email address.

I do admit it's quite an "extra" function, but it does provide the functionality our customers are looking for.

I did look at the idea of writing it manually, but that does mean, writing one function for these complex tables along side the one we already have that builds the query based on what's in the datatable.

Next, your sample code is too long to see and understand. Please write minimum code to reproduce it.

I suspected that might be the case. I will attempt to edit the queries down and will reply with some better examples of the error in time.

That new sub query addition might be what I need.
The current sub query documentation doesn't look like it allows for the checking of ID's across tables.
For example, where the sub query needs to check WHERE sub_query_id = main_query_id

If the new update will allow that, I might be able to resolve my problem anyway. I will give it a go this coming week.

Thanks for the helpful reply and I will write an update with new information shortly.

@kenjis
Copy link
Member

kenjis commented Mar 21, 2022

Thank you for reply.
But I don't understand the reason why you don't use query() method because of the language barrier.
If i were you, I don't use Query Builder for such a complex SQL statement.

I would like to ask you to:

  • show minimum sample code to reproduce one bug
    • if there are two bugs like one in select() and one in where(), create two issues
  • break a long line to see without scrolling

And as you know, Query Builder can't handle complex SQL string.
It might be a bug, but probably it is very difficult to fix. Because
the code is extremely difficult and complex, it came from the age of old CodeIgniter.
So please don't expect the bug will be fixed soon.

@kenjis
Copy link
Member

kenjis commented Mar 21, 2022

LIKE's problem was reported in #3970.

@kenjis kenjis added the duplicate Issue or pull request duplicates an already existing issue/pull request label Apr 2, 2022
@kenjis
Copy link
Member

kenjis commented Apr 2, 2022

@SupremeSimon
I believe this is duplicate of #3970.
So I close this.

If i am wrong, reopen or create new issue, please.

@kenjis kenjis closed this as completed Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer duplicate Issue or pull request duplicates an already existing issue/pull request
Projects
None yet
Development

No branches or pull requests

3 participants