Skip to content

Order line product link written as '' — breaks on MySQL STRICT_TRANS_TABLES #18

Description

@Pichinov-Jose

Repository: SplashSync/Dolibarr
Module version: 2.23.3 (also present in 2.21.0 — file unchanged between the two)
Dolibarr: 23.0.0 · PHP: 8.1 · DB: MariaDB with sql_mode=STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION

Summary

When an order line has no matching product (shipping lines, fees, discounts), setItemProductLink()
writes an empty string into the integer column llx_commandedet.fk_product. On any server running
in strict SQL mode the write is rejected and the line update fails.

Error

Splash\Local\Objects\Order::setItemProductLink() => Error :
  Incorrect integer value: '' for column `<db>`.`llx_commandedet`.`fk_product` at row 1

Repeated once per line whose product link has to be cleared. On our instance this made every
re-sync of an affected order fail, and the order could never be corrected again.

Root cause

src/Core/BaseItemsTrait.php:579

$productId = $product ? $product->id : null;
...
$this->currentItem->setValueFrom("fk_product", $productId, '', null, '', '', "none");

The 5th argument of CommonObject::setValueFrom() is $format. It is left empty, and Dolibarr
defaults it to 'text' (htdocs/core/class/commonobject.class.php):

if (empty($format)) {
    $format = 'text';
}
...
if ($format == 'text') {
    $sql .= $field." = '".$this->db->escape($value)."'";   // fk_product = ''
} elseif ($format == 'int') {
    $sql .= $field." = ".((int) $value);                   // fk_product = 0
}

So null is written as '' instead of 0. Non-strict servers silently coerce it to 0, which is
why this has gone unnoticed; strict servers reject it.

Steps to reproduce

  1. Set the database to STRICT_TRANS_TABLES.
  2. Sync an order containing a line with no matching product (a WooCommerce fee or shipping item).
  3. Re-sync the order so Splash has to clear an existing fk_product.

Suggested fix

-$this->currentItem->setValueFrom("fk_product", $productId, '', null, '', '', "none");
+$this->currentItem->setValueFrom("fk_product", $productId, '', null, 'int', '', "none");

(int) null === 0, which is Dolibarr's own convention for "no product" on a free-text line.

Impact

Silent on permissive servers, blocking on strict ones. Strict mode is the default on MySQL 5.7+
and on several managed hosts (this instance is on OVH PrivateSQL), so this is likely to affect
a growing number of installs.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions