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

Subquery with LIMIT in IN clause should be supported #1096

Open
mkrecek234 opened this issue Feb 24, 2023 · 12 comments
Open

Subquery with LIMIT in IN clause should be supported #1096

mkrecek234 opened this issue Feb 24, 2023 · 12 comments

Comments

@mkrecek234
Copy link
Contributor

mkrecek234 commented Feb 24, 2023

UPDATE ignore everything below as reported very badly.

MySQL/MariaDB repro: #1096 (comment)

MySQL issue needed for workaround: TODO open an issue

MariaDB issue needed for workaround: https://jira.mariadb.org/browse/MDEV-32657


I have a model that is a bit special, as we have a Customer model, with a hasOne('default_customer_contact_id') and a hasMany(CustomerContact). The default_contact_id is telling which of the multiple contacts is standard. Obviously, this creates the issue, that the sub-query is also limited 0,9 in SQL.

If you use this model and set a limit (e.g. like in Crud's IPP), it renders an issue that LIMIT is not supported in MySQL inside IN statement.

Here is the problematic query:

select 
  `id`, 
  `last_name`, 
  `given_name`, 
  `customer_id`, 
  (
    select 
      `default_contact_id` 
    from 
      `customer` `_C_c_06aa3ddbf548` 
    where 
      (
        `is_deleted` = 0 
        and `id` = `_C_ed9e2d38ef19`.`customer_id`
      )
  ) `default_contact_id`, 
  (
    select 
      `default_invoice_contact_id` 
    from 
      `customer` `_C_c_06aa3ddbf548` 
    where 
      (
        `is_deleted` = 0 
        and `id` = `_C_ed9e2d38ef19`.`customer_id`
      )
  ) `default_invoice_contact_id`, 
  (
    select 
      `default_shipping_contact_id` 
    from 
      `customer` `_C_c_06aa3ddbf548` 
    where 
      (
        `is_deleted` = 0 
        and `id` = `_C_ed9e2d38ef19`.`customer_id`
      )
  ) `default_shipping_contact_id`, 
  (
    select 
      (
        CONCAT(
          `matchcode`, " (# ", `customer_no`, 
          ") "
        )
      ) `name` 
    from 
      `customer` `_C_c_06aa3ddbf548` 
    where 
      (
        `is_deleted` = 0 
        and `id` = `_C_ed9e2d38ef19`.`customer_id`
      )
  ) `customer`, 
  `address_id`, 
  (
    select 
      `company` 
    from 
      `customer_address` `_C_a_9e7dd535952f` 
    where 
      (
        `customer_id` in (
          select 
            `id` 
          from 
            `customer` `_C_c_06aa3ddbf548` 
          where 
            (
              `is_deleted` = 0 
              and `id` in (
                select 
                  `customer_id` 
                from 
                  `customer_contact` `_C_ed9e2d38ef19` 
                where 
                  `customer_id` = 1016981 
                order by 
                  (
                    CONCAT_WS(" ", `given_name`, `last_name`)
                  ) 
                limit 
                  0, 
                  9
              )
            ) 
          order by 
            `LTM_sales` desc, 
            `customer_status_id`, 
            (
              CONCAT(
                `matchcode`, " (# ", `customer_no`, 
                ") "
              )
            )
        ) 
        and `id` = `_C_ed9e2d38ef19`.`address_id`
      )
  ) `company`, 
  `position`, 
  `department`, 
  `email`, 
  `phone`, 
  `mobile`, 
  `fax`, 
  `birthday`, 
  `note`, 
  `language_id`, 
  (
    REGEXP_REPLACE(`phone`, "[ ,(,),+,-]", "")
  ) `phone_clean`, 
  (
    REGEXP_REPLACE(`mobile`, "[ ,(,),+,-]", "")
  ) `mobile_clean`, 
  (
    `id` = (
      select 
        `default_contact_id` 
      from 
        `customer` `_C_c_06aa3ddbf548` 
      where 
        (
          `is_deleted` = 0 
          and `id` = `_C_ed9e2d38ef19`.`customer_id`
        )
    )
  ) `default_contact`, 
  (
    `id` = (
      select 
        `default_shipping_contact_id` 
      from 
        `customer` `_C_c_06aa3ddbf548` 
      where 
        (
          `is_deleted` = 0 
          and `id` = `_C_ed9e2d38ef19`.`customer_id`
        )
    ) 
    OR (
      (
        select 
          `default_shipping_contact_id` 
        from 
          `customer` `_C_c_06aa3ddbf548` 
        where 
          (
            `is_deleted` = 0 
            and `id` = `_C_ed9e2d38ef19`.`customer_id`
          )
      ) IS NULL 
      AND `id` = (
        select 
          `default_contact_id` 
        from 
          `customer` `_C_c_06aa3ddbf548` 
        where 
          (
            `is_deleted` = 0 
            and `id` = `_C_ed9e2d38ef19`.`customer_id`
          )
      )
    )
  ) `default_shipping_contact`, 
  (
    `id` = (
      select 
        `default_invoice_contact_id` 
      from 
        `customer` `_C_c_06aa3ddbf548` 
      where 
        (
          `is_deleted` = 0 
          and `id` = `_C_ed9e2d38ef19`.`customer_id`
        )
    ) 
    OR (
      (
        select 
          `default_invoice_contact_id` 
        from 
          `customer` `_C_c_06aa3ddbf548` 
        where 
          (
            `is_deleted` = 0 
            and `id` = `_C_ed9e2d38ef19`.`customer_id`
          )
      ) IS NULL 
      AND `id` = (
        select 
          `default_contact_id` 
        from 
          `customer` `_C_c_06aa3ddbf548` 
        where 
          (
            `is_deleted` = 0 
            and `id` = `_C_ed9e2d38ef19`.`customer_id`
          )
      )
    )
  ) `default_invoice_contact`, 
  (
    CONCAT_WS(
      ",", 
      IF(
        (
          `id` = (
            select 
              `default_contact_id` 
            from 
              `customer` `_C_c_06aa3ddbf548` 
            where 
              (
                `is_deleted` = 0 
                and `id` = `_C_ed9e2d38ef19`.`customer_id`
              )
          )
        ), 
        "Default", 
        ""
      ), 
      IF(
        (
          `id` = (
            select 
              `default_invoice_contact_id` 
            from 
              `customer` `_C_c_06aa3ddbf548` 
            where 
              (
                `is_deleted` = 0 
                and `id` = `_C_ed9e2d38ef19`.`customer_id`
              )
          ) 
          OR (
            (
              select 
                `default_invoice_contact_id` 
              from 
                `customer` `_C_c_06aa3ddbf548` 
              where 
                (
                  `is_deleted` = 0 
                  and `id` = `_C_ed9e2d38ef19`.`customer_id`
                )
            ) IS NULL 
            AND `id` = (
              select 
                `default_contact_id` 
              from 
                `customer` `_C_c_06aa3ddbf548` 
              where 
                (
                  `is_deleted` = 0 
                  and `id` = `_C_ed9e2d38ef19`.`customer_id`
                )
            )
          )
        ), 
        "Invoice", 
        NULL
      ), 
      IF(
        (
          `id` = (
            select 
              `default_shipping_contact_id` 
            from 
              `customer` `_C_c_06aa3ddbf548` 
            where 
              (
                `is_deleted` = 0 
                and `id` = `_C_ed9e2d38ef19`.`customer_id`
              )
          ) 
          OR (
            (
              select 
                `default_shipping_contact_id` 
              from 
                `customer` `_C_c_06aa3ddbf548` 
              where 
                (
                  `is_deleted` = 0 
                  and `id` = `_C_ed9e2d38ef19`.`customer_id`
                )
            ) IS NULL 
            AND `id` = (
              select 
                `default_contact_id` 
              from 
                `customer` `_C_c_06aa3ddbf548` 
              where 
                (
                  `is_deleted` = 0 
                  and `id` = `_C_ed9e2d38ef19`.`customer_id`
                )
            )
          )
        ), 
        "Shipping", 
        NULL
      )
    )
  ) `default`, 
  (
    CONCAT_WS(" ", `given_name`, `last_name`)
  ) `contact_name`, 
  (
    CONCAT_WS(" ", `given_name`, `last_name`)
  ) `name` 
from 
  `customer_contact` `_C_ed9e2d38ef19` 
where 
  `customer_id` = 1016981 
order by 
  (
    CONCAT_WS(" ", `given_name`, `last_name`)
  ) 
limit 
  0, 
  9

(query reformatted using https://codebeautify.org/sqlformatter)

@mvorisek
Copy link
Member

atk4/ui is very deeply tested incl. MySQL 8.0 :)

this issue needs a minimal repro code with model and ui code

@mvorisek
Copy link
Member

MySQL can be supported with a workaround - https://dbfiddle.uk/DpsuCdHc (and implemented in atk4/data to be applied automatically when needed)

still need a minimalistic repro to understand how this relates with CardDeck, as the CardDeck limit is probably not needed/wanted to be applied to any subquery

@mkrecek234
Copy link
Contributor Author

@mvorisek As corrected above, it is not a CardDeck issue, but a Atk4/Data issue which renders incorrect queries when combining the above hasOne / hasMany setup, so:

$parentModel->hasOne('default_contact_id, ['model' => [Contact::class]]);
$parentModel->hasMany('contacts', ['model' => [Contact::class]]);

$contactModel->hasOne('parent_id', ['model' => [Parent::class]])->addField('default_contact_id');
$contactModel->setLimit(5);

The rendered SQL for the $contactModel then is not compatible to MySQL.

@mvorisek mvorisek transferred this issue from atk4/ui Mar 4, 2023
@mvorisek
Copy link
Member

mvorisek commented Mar 4, 2023

@mkrecek234 please post here a complete model to be able to reproduce it

@mvorisek mvorisek changed the title CardDeck IPP not working in MySQL 8.0 Limit is wrongly used for inner query Mar 4, 2023
@mvorisek
Copy link
Member

mvorisek commented Mar 7, 2023

@mkrecek234 I would like this to be investigated, can you please provide the complete repro?

@mkrecek234
Copy link
Contributor Author

mkrecek234 commented Mar 7, 2023

@mvorisek Please check commit https://github.com/atk4/ui/tree/bug_data_nested_limit - you need to change database from SQlite to MySQL.

UPDATE: repro branch above was deleted, here is repro as patch:

diff --git a/demos/collection/crud.php b/demos/collection/crud.php
index d11fdf4..9822390 100644
--- a/demos/collection/crud.php
+++ b/demos/collection/crud.php
@@ -20,10 +20,23 @@ require_once __DIR__ . '/../init-app.php';
 
 $model = new Product($app->db);
 
+$crud = Crud::addTo($app, ['ipp' => 2]);
+$crud->addItemsPerPageSelector([2,20,50]);
+$crud->setModel($model);
+
+$model = new Category($app->db);
+$entity =  $model->load(3);
+
+$cardDeck = \Atk4\Ui\CardDeck::addTo($app, ['ipp' => 2]);
+$cardDeck->setModel($entity->ref($entity->fieldName()->Products));
+
+$model = new Category($app->db);
+
 $crud = Crud::addTo($app, ['ipp' => 5]);
 $crud->addItemsPerPageSelector([10,20,50]);
 $crud->setModel($model);
 
+
 $model = new Country($app->db);
 
 $crud = Crud::addTo($app, ['ipp' => 10]);
diff --git a/demos/init-db.php b/demos/init-db.php
index a3def9f..cb5ae65 100644
--- a/demos/init-db.php
+++ b/demos/init-db.php
@@ -445,7 +445,7 @@ class Category extends ModelWithPrefixedFields
         ]);
         $this->hasOne($this->fieldName()->default_product_id, [
             'model' => [Product::class],
-        ]);
+        ])->addTitle();
     }
 }
 
@@ -496,14 +496,20 @@ class Product extends ModelWithPrefixedFields
             'model' => [Category::class],
         ])->addFields([$this->product_category_id->fieldName()->default_product_id])->addTitle();
 
-        $this->hasOne($this->fieldName()->product_sub_category_id, [
+      /*  $this->hasOne($this->fieldName()->product_sub_category_id, [
             'model' => [SubCategory::class],
         ])->addTitle();
+*/
+
+        $this->hasOne($this->fieldName()->product_sub_category_id, ['model' => function($m) {
+            return $m->product_category_id->SubCategories;
+        }])->addField($this->product_category_id->SubCategories->fieldName()->name);
+
         $this->addExpression($this->fieldName()->isDefault, [
             'expr' => function (Model /* TODO self is not working bacause of clone in Multiline */ $row) {
-                return $row->expr('{' . $this->product_category_id->fieldName()->default_product_id . '}'); // @phpstan-ignore-line
+                return $row->expr('{' . $this->product_category_id->fieldName()->default_product_id . '} = {'. $this->fieldName()->id.'}'); // @phpstan-ignore-line
             },
-            'type' => 'integer',
+            'type' => 'boolean',
         ]);
     }
 }

@mvorisek
Copy link
Member

mvorisek commented Mar 17, 2023

@mkrecek234 please always post a minimal repro and complete repro steps. It saves my times and increase the chance the issue will be investigated/fixed.

I checkout the repro branch, recreated a MySQL DB and opened demos/collection/crud.php. What steps are needed to reproduce? (tested with MySQL 5.7 / Win / PHP 7.4)

@mvorisek
Copy link
Member

@mkrecek234 please provide the steps to reproduce

@mvorisek
Copy link
Member

@mkrecek234 can you please reply to this issue?

@mkrecek234
Copy link
Contributor Author

mkrecek234 commented Aug 17, 2023

@mvorisek I suggest to close this issue as this is a very special model structure which causes it. If you're interested, here it is:

  • I have a customer model which has two child models CustomerAddress (exact Company name, street, city) and CustomerContact (last name, given name, phone).
  • It also has two hasOne relations, 'default_address_id' and default_contact_id which refer to a single entity of each of those two child tables, marking which is the standard address or contact.
  • Furthermore, each contact (last name, given name etc.) in CustomerContact can be assigned also any of the customer's address records. To be sure that you can only assign an address to a contact which belongs to the same customer, I have created the following relation in CustomerContact:
            $this->hasOne('address_id', ['model' => function($m) {
                return $m->ref('customer_id')->ref('CustomerAddress');
            }])->addField('company');

This way in case you show all CustomerAddress records of a given customer in a crud or deck with paginators, the above LIMIT bug in SQL is created.

However, I found a simple workaround to make this work again:

            $this->hasOne('address_id', ['model' => function($m) {
                 return $m->setLimit(null)->ref('customer_id')->ref('CustomerAddress');
            }])->addField('company');

By this, the inner setLimit will be eliminated, and the query is correctly rendered.

Because of this very special model structure, I did not find a quick way to generate a replicable sample code easily today. Thus, I leave it like that and would prefer to close this issue.

@mvorisek
Copy link
Member

Limit in subquery is fully supported by Sqlite/MySQL. It is problematic in MSSQL/Oracle, but that is not your case.

Given this, if there is some issue, it should be fixed/is fixable.

If I understood your issue well, although I did not spent much time to read everything, the issue is about some atk4/ui component setting LIMIT which is then used for some inner query. If this is true, this meants it is an atk4/ui issue - please confirm and I can migrate it to atk4/ui repo.

And please provide full repro. If you need some models, reuse the ones from atk4/ui demos. Then the repro should be below 10 LoC. Thank you.

@mvorisek mvorisek reopened this Aug 17, 2023
@mvorisek mvorisek changed the title Limit is wrongly used for inner query Subquery with LIMIT in IN clause should be supported Nov 5, 2023
@mvorisek
Copy link
Member

mvorisek commented Nov 5, 2023

In order to support this, https://jira.mariadb.org/browse/MDEV-32657 must be fixed first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants