Ensure that jdbc temp tables can be dropped if the insertions fail

[hackeryang]

Description

We use Trino to query and insert mysql tables, and one day we found that our MySQL appeared many temp tables which were created long time ago as shown below:

mysql> show tables like 'tmp_trino_%';
   ......

| tmp_trino_fc833837bf0a4a0baec24baad3a2ddc0 |
| tmp_trino_fc8cc1be908142768c406450056d7132 |
| tmp_trino_fd6db0d078c048969758952a0cb5ecb3 |
| tmp_trino_fd8c62fc3b2443e5b06a27763f293f71 |
| tmp_trino_fe25d4a35c5c4033929cd7bafc8a09e9 |
| tmp_trino_feb573909ffd4e9a85cd5d7a74eb319e |
| tmp_trino_feb6e4bfb7f447c5a098eb6576a6c0d2 |
| tmp_trino_fefff60390e64f8988fb2a6cfaf23434 |
| tmp_trino_ff0bce6650c343ecb9c6e99b57d851f1 |
| tmp_trino_ff7ded0490974df1a46443777c1a5e9b |
| tmp_trino_ff93a57fc58447d2b4cf776ca3967774 |
+--------------------------------------------+
964 rows in set (0.00 sec)

mysql> desc tmp_trino_fefff60390e64f8988fb2a6cfaf23434;
+--------------------+------------+------+-----+---------+-------+
| Field              | Type       | Null | Key | Default | Extra |
+--------------------+------------+------+-----+---------+-------+
| trino_page_sink_id | bigint(20) | YES  |     | NULL    |       |
+--------------------+------------+------+-----+---------+-------+
1 row in set (0.00 sec)

After reading relevant source codes, we found that this place may cause a bug:


If the execution of the statement pageSinkInsertSql fails in constructPageSinkIdsTable(), the closer.register(() -> dropTable(session, pageSinkTable, true)) in finishInsertTable() won't be executed, and will directly jump to the finally code block to execute closer.close(), which only has closer.register(() -> dropTable(session, temporaryTable, true)) without pageSinkTable.

So we need to register the drop table closer before the execution of insertion statements.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Ensure that jdbc temp tables can be dropped if the insertions fail. ({issue}`19368`)

[hackeryang]

Hello @mwd410 @losipiuk @kokosing , can you please take a review when you have time, thanks~

[kokosing]

Is it possible to have a test?

[hackeryang]

Is it possible to have a test?

@kokosing Sorry i didn't figure out a good way to mock the jdbc insertion failure, it seems that we need to implement java.sql.Statement.executeBatch(). I read the codes in TestBaseJdbcClient and did not find some APIs which can be used to simulate the insertion error

[hackeryang]

Hi @kokosing @hashhar , it seems that other tests have been passed, can you please help merge when you have time, thank you~

[wendigo]

Thanks @hackeryang