projection containing virtual columns should be evaluated before the UnionScan(dirty write content)

[bb7133]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

CREATE TABLE `gholla_dummy1` (
  `id` varchar(10) NOT NULL,
  `mark` int,
  `deleted_at` datetime(3) NOT NULL DEFAULT '1970-01-01 01:00:01.000',
  `account_id` varchar(10) NOT NULL,
  `metastore_id` varchar(10) NOT NULL,
  `is_deleted` tinyint(1) GENERATED ALWAYS AS ((`deleted_at` > _utf8mb4'1970-01-01 01:00:01.000')) VIRTUAL NOT NULL,
  PRIMARY KEY (`account_id`,`metastore_id`,`id`),
  KEY `isDeleted_accountId_metastoreId` (`is_deleted`,`account_id`,`metastore_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;

CREATE TABLE `gholla_dummy2` (
  `id` varchar(10) NOT NULL,
  `mark` int,
  `deleted_at` datetime(3) NOT NULL DEFAULT '1970-01-01 01:00:01.000',
  `account_id` varchar(10) NOT NULL,
  `metastore_id` varchar(10) NOT NULL,
  `is_deleted` tinyint(1) GENERATED ALWAYS AS ((`deleted_at` > _utf8mb4'1970-01-01 01:00:01.000')) VIRTUAL NOT NULL,
  PRIMARY KEY (`account_id`,`metastore_id`,`id`),
  KEY `isDeleted_accountId_metastoreId` (`is_deleted`,`account_id`,`metastore_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin; 

INSERT INTO gholla_dummy1 (id,mark,deleted_at,account_id,metastore_id) VALUES ('ABC', 1, '1970-01-01 01:00:01.000', 'ABC', 'ABC');

INSERT INTO gholla_dummy2 (id,mark,deleted_at,account_id,metastore_id) VALUES ('ABC', 1, '1970-01-01 01:00:01.000', 'ABC', 'ABC');

start transaction;
update gholla_dummy2 set deleted_at = NOW(), mark=2 where account_id = 'ABC' and metastore_id = 'ABC' and id = 'ABC';
select
  /*+ INL_JOIN(g1, g2) */
  g1.account_id,
  g2.mark
from
  gholla_dummy1 g1 FORCE INDEX(isDeleted_accountId_metastoreId) 
STRAIGHT_JOIN
  gholla_dummy2 g2 FORCE INDEX (PRIMARY)
ON
  g1.account_id = g2.account_id AND
  g1.metastore_id = g2.metastore_id AND
  g1.id = g2.id
WHERE
  g1.account_id = 'ABC' AND
  g1.metastore_id = 'ABC' AND
  g1.is_deleted = FALSE AND
  g2.is_deleted = FALSE;

2. What did you expect to see? (Required)

No result was returned because we updated is_deleted in the transaction.

3. What did you see instead (Required)

tidb> select
    ->   /*+ INL_JOIN(g1, g2) */
    ->   g1.account_id,
    ->   g2.mark
    -> from
    ->   gholla_dummy1 g1 FORCE INDEX(isDeleted_accountId_metastoreId)
    -> STRAIGHT_JOIN
    ->   gholla_dummy2 g2 FORCE INDEX (PRIMARY)
    -> ON
    ->   g1.account_id = g2.account_id AND
    ->   g1.metastore_id = g2.metastore_id AND
    ->   g1.id = g2.id
    -> WHERE
    ->   g1.account_id = 'ABC' AND
    ->   g1.metastore_id = 'ABC' AND
    ->   g1.is_deleted = FALSE AND
    ->   g2.is_deleted = FALSE;
+------------+------+
| account_id | mark |
+------------+------+
| ABC        |    1 |
+------------+------+
1 row in set (0.01 sec)

The update in the transaction was not read.

4. What is your TiDB version? (Required)

It exists in all versions of TiDB.

[bb7133]

If we check the execution plans, it looks fine:

tidb> explain select   /*+ INL_JOIN(g1, g2) */   g1.account_id,   g2.mark from   gholla_dummy1 g1 FORCE INDEX(isDeleted_accountId_metastoreId)  STRAIGHT_JOIN   gholla_dummy2 g2 FORCE INDEX (PRIMARY) ON   g1.account_id = g2.account_id AND   g1.metastore_id = g2.metastore_id AND   g1.id = g2.id WHERE   g1.account_id = 'ABC' AND   g1.metastore_id = 'ABC' AND   g1.is_deleted = FALSE AND   g2.is_deleted = FALSE;
+----------------------------------+---------+-----------+---------------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                               | estRows | task      | access object                                                                         | operator info                                                                                                                                                                                                                                                                                                                                                                                                                         |
+----------------------------------+---------+-----------+---------------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| IndexJoin_15                     | 0.00    | root      |                                                                                       | inner join, inner:UnionScan_14, outer key:test.gholla_dummy1.account_id, test.gholla_dummy1.metastore_id, test.gholla_dummy1.id, inner key:test.gholla_dummy2.account_id, test.gholla_dummy2.metastore_id, test.gholla_dummy2.id, equal cond:eq(test.gholla_dummy1.account_id, test.gholla_dummy2.account_id), eq(test.gholla_dummy1.id, test.gholla_dummy2.id), eq(test.gholla_dummy1.metastore_id, test.gholla_dummy2.metastore_id) |
| ├─IndexReader_29(Build)          | 1.00    | root      |                                                                                       | index:IndexRangeScan_28                                                                                                                                                                                                                                                                                                                                                                                                               |
| │ └─IndexRangeScan_28            | 1.00    | cop[tikv] | table:g1, index:isDeleted_accountId_metastoreId(is_deleted, account_id, metastore_id) | range:[0 "ABC" "ABC",0 "ABC" "ABC"], keep order:false, stats:pseudo                                                                                                                                                                                                                                                                                                                                                                   |
| └─UnionScan_14(Probe)            | 0.00    | root      |                                                                                       | eq(test.gholla_dummy2.account_id, "ABC"), eq(test.gholla_dummy2.is_deleted, 0), eq(test.gholla_dummy2.metastore_id, "ABC")                                                                                                                                                                                                                                                                                                            |
|   └─Selection_13                 | 0.00    | root      |                                                                                       | eq(test.gholla_dummy2.is_deleted, 0)                                                                                                                                                                                                                                                                                                                                                                                                  |
|     └─Projection_12              | 1.00    | root      |                                                                                       | test.gholla_dummy2.id, test.gholla_dummy2.mark, test.gholla_dummy2.account_id, test.gholla_dummy2.metastore_id, test.gholla_dummy2.is_deleted                                                                                                                                                                                                                                                                                         |
|       └─TableReader_11           | 0.00    | root      |                                                                                       | data:Selection_10                                                                                                                                                                                                                                                                                                                                                                                                                     |
|         └─Selection_10           | 0.00    | cop[tikv] |                                                                                       | eq(test.gholla_dummy2.account_id, "ABC"), eq(test.gholla_dummy2.metastore_id, "ABC")                                                                                                                                                                                                                                                                                                                                                  |
|           └─TableRangeScan_9     | 1.00    | cop[tikv] | table:g2                                                                              | range: decided by [eq(test.gholla_dummy2.account_id, test.gholla_dummy1.account_id) eq(test.gholla_dummy2.metastore_id, test.gholla_dummy1.metastore_id) eq(test.gholla_dummy2.id, test.gholla_dummy1.id)], keep order:false, stats:pseudo                                                                                                                                                                                            |
+----------------------------------+---------+-----------+---------------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

The data in the transaction buffer should be read by the UnionScan_14. However, when building the executors in buildUnionScanFromReader, it seems that we assume there will be only 'readers' as the children of Selection executors:

switch x := reader.(type) {
case *MPPGather:
...
case *TableReaderExecutor:
...
case *IndexReaderExecutor:
...
case *IndexLookUpExecutor:
...
case *IndexMergeReaderExecutor:
...
default:
    // The mem table will not be written by sql directly, so we can omit the union scan to avoid err reporting.
    return originReader
}

In this case, we have Projection_12 as the child of Selection_13 so the switch falls to default, and UnionScan executor was not built at all, that's why the data from the memory buffer is lost.

[bb7133]

Thinking about how to solve this issue, but:

1. I am not clear about the usage of Projection here, is it required?
2. The ugly fixing is to check the attributes of Projection and the child of it(...Readers), but this is nothing but a patch for this case. A more robust way is needed I believe.

[bb7133]

Thinking about how to solve this issue, but:

I am not clear about the usage of Projection here, is it required?
The ugly fixing is to check the attributes of Projection and the child of it(...Readers), but this is nothing but a patch for this case. A more robust way is needed I believe.

Update:

The Projection is used to prune the 'base column' of virtual generated column generated from TableReaderExecutor(in this case, it is column deleted_at, so it is required.

The codes shown in 'buildUnionScanFromReader' is old and vulnerable, in order to avoid the potential risks, we decide to report errors(instead of return wrong results) when facing 'unknown' executors: #53956.

To solve this bug in a short term, we prevent pushing Projection with VGC down to UnionScan in #53981, because it might lead to plans like UnionScan -> Selection -> Projection, which is actually not supported yet.

[vgkholla]

hi, @bb7133, what effect (if any) does preventing projection propagation have on performance? would it mean that more data might need to be shuttled through some layers inside TiDB/TiKV? if there are queries that select only "smaller" columns in a "large" row and use VGC in the where clause, would their performance worsen (because there are larger disk reads or more data to shuttle, etc)?

[seiya-annie]

/found customer

[winoros]

hi, @bb7133, what effect (if any) does preventing projection propagation have on performance? would it mean that more data might need to be shuttled through some layers inside TiDB/TiKV? if there are queries that select only "smaller" columns in a "large" row and use VGC in the where clause, would their performance worsen (because there are larger disk reads or more data to shuttle, etc)?

The fix will not cause performance problems currently.
The projection would also be executed on the TiDB side before the fix.

But, yes, we'll get a better performance if the projection can be calculated at the TiKV side when the transaction has un-committed data and if the projection has json related functions.

TiDB has some simple column pruning tech when reading data from TiKV even if no projection is pushed to TiKV.

[seiya-annie]

/found customer