FWD » Core Development » Database

TemporaryBuffer.removeRecords is widely used when closing a multiplex scope, but not only - e.g it is also used with copy-temp-table or empty-temp-table. The code looks quite complex, but I am sure it can be comprehended and refactored to use prepared statements:

delete from dtt1 where _multiplex = 1745

delete from dtt1 where _multiplex = ?

Note that we can use TemporaryBuffer.removeRecords with explicit where clause and args.

We should also address the use of prepared statements when using FastCopyHelper. Most of the statements executed there are prepared, but there are some places in which the multiplex is hard-coded FastCopyHelper.getSrcTempColumns, FastCopyHelper.getSrcMappedColumns. These are the methods used to define the columns of the source table for a INSERT-SELECT operation. The multiplex should be explicit to ensure that the records reach the target table with a specified multiplex. However, the multiplex is more then explicit; it is hard-coded.

Alexandru Lungu wrote:

TemporaryBuffer.removeRecords is widely used when closing a multiplex scope, but not only - e.g it is also used with copy-temp-table or empty-temp-table. The code looks quite complex, but I am sure it can be comprehended and refactored to use prepared statements:

I've refactored this. The arguments are stored separately from the ones received as a parameter to the method in order to ensure their correct order. Right before the statement is executed, they get merged.

We should also address the use of prepared statements when using FastCopyHelper. Most of the statements executed there are prepared, but there are some places in which the multiplex is hard-coded FastCopyHelper.getSrcTempColumns, FastCopyHelper.getSrcMappedColumns.

Done. I've added the multiplexID to the list of arguments and removed the "hard-coding operation".

Additional note: In FastCopyHelper.safeTableCopy I believe there was a typo. In order to call persistence.executeSQL with arguments, I don't think the condition args.isEmpty() is the one intended, so I just reversed it. It is very likely that this was solved on another issue, I just couldn't find it.

The changes were committed to 7061a, rev.14487 and tested with various examples and a large customer application.

Alexandru Lungu wrote:

Radu, please move 7061a, rev. 14487 to 7026b.
I will review, test and profile it once there.

The changes have been committed to 7026b, rev. 14501.

Review of 7026b/rev. 14501

• In TemporaryBuffer.java, add a proper default capacity for additionalArgs and ooArgs (maybe 2?). Otherwise, the array will be expended to 32 by default (too much for our needs).
• I am OK with the changes overall!

Tested 7026b/rev. 14503 and got -1.8% improvement overall.
There was no regression in the POCs I tested.

Alexandru Lungu wrote:

• In TemporaryBuffer.java, add a proper default capacity for additionalArgs and ooArgs (maybe 2?). Otherwise, the array will be expended to 32 by default (too much for our needs).

I've added an initial capacity to those lists. The change is on 7026b, rev. 14506.

This was merged in trunk as rev. 14523 and can be closed.