atk4 / data

Data Access PHP Framework for SQL & high-latency databases
https://atk4-data.readthedocs.io
MIT License
271 stars 46 forks source link

Subquery with LIMIT in IN clause should be supported #1096

Open mkrecek234 opened 1 year ago

mkrecek234 commented 1 year ago

UPDATE ignore everything below as reported very badly.

MySQL/MariaDB repro: https://github.com/atk4/data/issues/1096#issuecomment-1454742258

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 commented 1 year ago

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

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

mvorisek commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

mvorisek commented 1 year ago

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

mkrecek234 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@mkrecek234 please provide the steps to reproduce

mvorisek commented 1 year ago

@mkrecek234 can you please reply to this issue?

mkrecek234 commented 1 year ago

@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:

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 commented 1 year ago

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 commented 10 months ago

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