Commit a01f4ff473f for woocommerce
commit a01f4ff473f0b746e2fcf1093bef1ebd315fcd53
Author: Miroslav Mitev <m1r0@users.noreply.github.com>
Date: Mon Sep 7 16:50:37 2026 +0300
Fix Analytics tax reports counting a legacy lookup row twice (#68374)
* Fix Analytics tax reports counting a legacy lookup row twice
diff --git a/plugins/woocommerce/changelog/38347-tax-lookup-grain-schema b/plugins/woocommerce/changelog/38347-tax-lookup-grain-schema
index bfc93658e8a..ecd26b194bc 100644
--- a/plugins/woocommerce/changelog/38347-tax-lookup-grain-schema
+++ b/plugins/woocommerce/changelog/38347-tax-lookup-grain-schema
@@ -1,4 +1,4 @@
Significance: minor
Type: dev
-wc_order_tax_lookup gains an order_item_id column and is re-keyed on (order_id, tax_rate_id, order_item_id), so it now holds one row per tax line instead of one row per tax rate. Code summing the table is unaffected and code counting rows will see more of them. Code writing to the table with $wpdb->replace() on (order_id, tax_rate_id) lands on the new column's zero default, which no longer replaces the rows WooCommerce writes for that order, so both are counted and the order's tax is reported twice; such code needs to write order_item_id too. Existing rows are rebuilt in the background, and can be rebuilt by hand from WooCommerce > Status > Tools.
+wc_order_tax_lookup gains an order_item_id column and is re-keyed on (order_id, tax_rate_id, order_item_id), so it now holds one row per tax line instead of one row per tax rate. Code summing the table is unaffected and code counting rows will see more of them. Code writing to the table with $wpdb->replace() on (order_id, tax_rate_id) lands on the new column's zero default rather than replacing the rows WooCommerce writes for that order. The reports leave such a row out while those rows stand, so its values are not read at all and it needs to write order_item_id too. Existing rows are rebuilt in the background, and can be rebuilt by hand from WooCommerce > Status > Tools.
diff --git a/plugins/woocommerce/src/Admin/API/Reports/Taxes/DataStore.php b/plugins/woocommerce/src/Admin/API/Reports/Taxes/DataStore.php
index 4bf24e42e53..862d403a123 100644
--- a/plugins/woocommerce/src/Admin/API/Reports/Taxes/DataStore.php
+++ b/plugins/woocommerce/src/Admin/API/Reports/Taxes/DataStore.php
@@ -170,10 +170,11 @@ class DataStore extends ReportsDataStore implements DataStoreInterface {
*
* Rows recorded before the lookup held one row per tax order item sit at the column's zero
* default and have no line to narrow to, so they go on matching the rate id alone, which is
- * how the report read them all along. OrderTaxLookupMigrator rebuilds them in the
+ * how the report read them all along, for as long as no per-line row has replaced them.
+ * See get_legacy_row_condition(). OrderTaxLookupMigrator rebuilds them in the
* background.
*/
- $this->subquery->add_sql_clause( 'where', "AND ( {$order_tax_lookup_table}.order_item_id = 0 OR {$order_tax_lookup_table}.order_item_id = {$wpdb->prefix}woocommerce_order_items.order_item_id )" );
+ $this->subquery->add_sql_clause( 'where', "AND ( {$order_tax_lookup_table}.order_item_id = {$wpdb->prefix}woocommerce_order_items.order_item_id OR " . self::get_legacy_row_condition() . ' )' );
if ( isset( $query_args['taxes'] ) && ! empty( $query_args['taxes'] ) ) {
$allowed_taxes = self::get_filtered_ids( $query_args, 'taxes' );
@@ -332,6 +333,48 @@ class DataStore extends ReportsDataStore implements DataStoreInterface {
return self::$lookup_keyed_by_order_item;
}
+ /**
+ * SQL condition matching a lookup row at the tax order item column's zero default that the
+ * reports should still read.
+ *
+ * A row at zero stands in for every tax line of its order that shares its rate, which is how
+ * the report read the lookup before the column existed. Rows written per tax line say the same
+ * thing line by line, so an order holding both shapes for one rate reports that rate twice.
+ * The per-line rows are the ones the sync maintains, so a legacy row they already cover is
+ * left out.
+ *
+ * A store ends up holding both shapes when code outside WooCommerce writes the lookup on the
+ * (order_id, tax_rate_id) key the table was released with: such a write lands on the zero
+ * default instead of replacing the rows WooCommerce holds for the order. A prune that could
+ * not run in `sync_order_taxes()` leaves the same shape behind.
+ *
+ * Once the lookup is keyed by tax order item it only holds rows at zero until the rebuild has
+ * been through it, so the check costs nothing on a store that is through it:
+ * `order_item_id > 0` is true and the subquery is never reached. It is paid for while the
+ * rebuild runs, where every row at zero takes an index lookup to find no per-line row beside
+ * it. Measured on a 2M row lookup over a 30 day report, that is 177ms against 595ms, and it
+ * goes away as the rebuild does.
+ *
+ * @internal For exclusive usage of WooCommerce core, backwards compatibility not guaranteed.
+ * @since 11.2.0
+ *
+ * @return string
+ */
+ public static function get_legacy_row_condition(): string {
+ $table_name = self::get_db_table_name();
+
+ // Nothing can have replaced a row while the lookup is still keyed on
+ // (order_id, tax_rate_id): the sync writes every row at zero and `OrderTaxLookupMigrator`
+ // waits for the re-key. Unlike a rebuild that is still queued, that state does not clear
+ // itself, so the subquery would read the report range on every uncached report and come
+ // back empty for good.
+ if ( ! self::lookup_is_keyed_by_order_item() ) {
+ return "{$table_name}.order_item_id = 0";
+ }
+
+ return "( {$table_name}.order_item_id = 0 AND NOT EXISTS ( SELECT 1 FROM {$table_name} per_line WHERE per_line.order_id = {$table_name}.order_id AND per_line.tax_rate_id = {$table_name}.tax_rate_id AND per_line.order_item_id > 0 ) )";
+ }
+
/**
* Create or update an entry in the wc_order_tax_lookup table for an order.
*
diff --git a/plugins/woocommerce/src/Admin/API/Reports/Taxes/Stats/DataStore.php b/plugins/woocommerce/src/Admin/API/Reports/Taxes/Stats/DataStore.php
index c7b20689caa..1c8b8cc661c 100644
--- a/plugins/woocommerce/src/Admin/API/Reports/Taxes/Stats/DataStore.php
+++ b/plugins/woocommerce/src/Admin/API/Reports/Taxes/Stats/DataStore.php
@@ -9,6 +9,7 @@ defined( 'ABSPATH' ) || exit;
use Automattic\WooCommerce\Admin\API\Reports\DataStore as ReportsDataStore;
use Automattic\WooCommerce\Admin\API\Reports\DataStoreInterface;
+use Automattic\WooCommerce\Admin\API\Reports\Taxes\DataStore as TaxesDataStore;
use Automattic\WooCommerce\Admin\API\Reports\TimeInterval;
use Automattic\WooCommerce\Admin\API\Reports\StatsDataStoreTrait;
@@ -110,7 +111,11 @@ class DataStore extends ReportsDataStore implements DataStoreInterface {
// (date_paid by default) rather than the lookup's date_created, so the Taxes report
// reconciles with the Orders and Revenue reports.
$this->add_time_period_sql_params( $query_args, $order_stats_table );
- $taxes_where_clause = '';
+
+ // This report sums the lookup rows as they stand, so a row a per-line row has replaced
+ // would be added on top of it and report its rate twice.
+ $taxes_where_clause = " AND ( {$order_tax_lookup_table}.order_item_id > 0 OR " . TaxesDataStore::get_legacy_row_condition() . ' )';
+
$order_status_filter = $this->get_status_subquery( $query_args );
if ( isset( $query_args['taxes'] ) && ! empty( $query_args['taxes'] ) ) {
diff --git a/plugins/woocommerce/tests/php/src/Admin/API/Reports/Taxes/DataStoreTest.php b/plugins/woocommerce/tests/php/src/Admin/API/Reports/Taxes/DataStoreTest.php
index a0afcce696f..8833ce72de8 100644
--- a/plugins/woocommerce/tests/php/src/Admin/API/Reports/Taxes/DataStoreTest.php
+++ b/plugins/woocommerce/tests/php/src/Admin/API/Reports/Taxes/DataStoreTest.php
@@ -969,6 +969,22 @@ class DataStoreTest extends WC_Unit_Test_Case {
ReportsCache::invalidate();
}
+ /**
+ * Point the Taxes data store at a table of the given name and forget what
+ * `DataStore::lookup_is_keyed_by_order_item()` read, so that the new table's key is seen.
+ *
+ * @param string $table_name Table name, without the database prefix.
+ */
+ private function point_data_store_at( string $table_name ): void {
+ $table = new \ReflectionProperty( DataStore::class, 'table_name' );
+ $table->setAccessible( true );
+ $table->setValue( null, $table_name );
+
+ $cache = new \ReflectionProperty( DataStore::class, 'lookup_keyed_by_order_item' );
+ $cache->setAccessible( true );
+ $cache->setValue( null, null );
+ }
+
/**
* Two tax lines on distinct rate ids, the shape almost every store's history is in.
*
@@ -1042,4 +1058,131 @@ class DataStoreTest extends WC_Unit_Test_Case {
$this->assertSame( array( 0.25, 0.25, 6.0, 6.0 ), $amounts, 'No line should be counted twice or lost.' );
$this->assertSame( 12.5, array_sum( $amounts ), 'The report should add up to the tax both orders carry.' );
}
+
+ /**
+ * Write a lookup row the way code outside WooCommerce did while the table was keyed on
+ * (order_id, tax_rate_id). Such a write now lands on the tax order item column's zero default
+ * instead of replacing the rows WooCommerce holds for the order.
+ *
+ * @param int $order_id Order id.
+ * @param int $rate_id Tax rate id.
+ * @param float $total_tax Tax the row carries.
+ */
+ private function write_lookup_row_on_the_released_key( int $order_id, int $rate_id, float $total_tax ): void {
+ global $wpdb;
+
+ $wpdb->replace(
+ $wpdb->prefix . 'wc_order_tax_lookup',
+ array(
+ 'order_id' => $order_id,
+ 'tax_rate_id' => $rate_id,
+ 'date_created' => '2023-02-10 10:00:00',
+ 'shipping_tax' => 0,
+ 'order_tax' => $total_tax,
+ 'total_tax' => $total_tax,
+ ),
+ array( '%d', '%d', '%s', '%f', '%f', '%f' )
+ );
+
+ ReportsCache::invalidate();
+ }
+
+ /**
+ * @testdox Taxes report leaves out a lookup row written on the released (order_id, tax_rate_id) key beside the rows the order already holds.
+ */
+ public function test_taxes_report_leaves_out_a_lookup_row_written_on_the_released_key(): void {
+ update_option( 'woocommerce_date_type', 'date_paid' );
+ WC_Helper_Reports::reset_stats_dbs();
+
+ $order = $this->seed_order_with_tax_lines( $this->tax_lines_on_distinct_rate_ids( 'US-CA', 101 ), '2023-02-10 10:00:00', '2023-02-10 10:00:00' );
+
+ $this->write_lookup_row_on_the_released_key( $order->get_id(), 101, 6.0 );
+
+ $sut = new DataStore();
+ $data = $sut->get_data( $this->all_taxes_query( '2023-02-01 00:00:00', '2023-02-28 23:59:59' ) );
+
+ $amounts = array_column( $data->data, 'total_tax' );
+ sort( $amounts );
+ $this->assertSame( array( 0.25, 6.0 ), $amounts, 'The order should report the tax it carries, not that tax plus the row left standing beside it.' );
+ }
+
+ /**
+ * @testdox Taxes stats leave out a lookup row written on the released (order_id, tax_rate_id) key beside the rows the order already holds.
+ */
+ public function test_taxes_stats_leave_out_a_lookup_row_written_on_the_released_key(): void {
+ update_option( 'woocommerce_date_type', 'date_paid' );
+ WC_Helper_Reports::reset_stats_dbs();
+
+ $order = $this->seed_order_with_tax_lines( $this->tax_lines_on_distinct_rate_ids( 'US-CA', 101 ), '2023-02-10 10:00:00', '2023-02-10 10:00:00' );
+
+ $this->write_lookup_row_on_the_released_key( $order->get_id(), 101, 6.0 );
+
+ $sut = new StatsDataStore();
+ $data = $sut->get_data( $this->all_taxes_query( '2023-02-01 00:00:00', '2023-02-28 23:59:59' ) + array( 'interval' => 'day' ) );
+
+ $this->assertSame( 6.25, $data->totals->total_tax, 'The stats total should match the tax the order carries.' );
+ }
+
+ /**
+ * @testdox Taxes stats still count rows written before the lookup was keyed by tax order item.
+ */
+ public function test_taxes_stats_read_rows_written_before_the_grain_change(): void {
+ update_option( 'woocommerce_date_type', 'date_paid' );
+ WC_Helper_Reports::reset_stats_dbs();
+
+ $order = $this->seed_order_with_tax_lines( $this->tax_lines_on_distinct_rate_ids( 'US-CA', 101 ), '2023-02-10 10:00:00', '2023-02-10 10:00:00' );
+
+ // Nothing has replaced these rows, so leaving them out would drop the order's tax from the
+ // report until the rebuild has been through it.
+ $this->unmigrate_lookup_rows( $order->get_id() );
+
+ $sut = new StatsDataStore();
+ $data = $sut->get_data( $this->all_taxes_query( '2023-02-01 00:00:00', '2023-02-28 23:59:59' ) + array( 'interval' => 'day' ) );
+
+ $this->assertSame( 6.25, $data->totals->total_tax, 'Rows waiting on the rebuild should still be counted.' );
+ }
+
+ /**
+ * @testdox Taxes reports drop the replaced-row check while the lookup is not keyed by tax order item.
+ */
+ public function test_taxes_reports_drop_the_replaced_row_check_while_the_lookup_is_not_re_keyed(): void {
+ global $wpdb;
+
+ $unkeyed_table = $wpdb->prefix . 'wc_order_tax_lookup_unkeyed';
+
+ // The shape a failed re-key leaves behind: dbDelta added the column, the key change never
+ // landed. Nothing writes a per-line row there, so no row can have been replaced.
+ //
+ // The store reads the shape of its table rather than its rows, so a temporary table in
+ // that shape stands in for the lookup. Re-keying the real one would need `ALTER TABLE`,
+ // which commits the transaction the test framework rolls back after each test, leaving
+ // both the key change and whatever else this class had written standing for the rest of
+ // the run. Temporary table DDL commits nothing.
+ /* phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Table name is not user input. */
+ $wpdb->query(
+ "CREATE TEMPORARY TABLE `{$unkeyed_table}` (
+ order_id bigint(20) unsigned NOT NULL,
+ tax_rate_id bigint(20) unsigned NOT NULL,
+ order_item_id bigint(20) unsigned NOT NULL DEFAULT 0,
+ PRIMARY KEY (order_id, tax_rate_id)
+ )"
+ );
+ /* phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching */
+
+ try {
+ $this->point_data_store_at( 'wc_order_tax_lookup_unkeyed' );
+
+ $condition = DataStore::get_legacy_row_condition();
+
+ $this->assertStringNotContainsString( 'NOT EXISTS', $condition, 'The reports should not look for a per-line row that cannot be there.' );
+ $this->assertSame( "{$unkeyed_table}.order_item_id = 0", $condition, 'The reports should read a row at zero the way the released report did.' );
+ } finally {
+ $this->point_data_store_at( 'wc_order_tax_lookup' );
+
+ /* phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Table name is not user input. */
+ $wpdb->query( "DROP TEMPORARY TABLE `{$unkeyed_table}`" );
+ }
+
+ $this->assertStringContainsString( 'NOT EXISTS', DataStore::get_legacy_row_condition(), 'The check should be back once the re-key has landed.' );
+ }
}