Add configurable insert_batch_size JDBC session property

[sergey-melnychuk]

Example: set session postgresql.insert_batch_size = 42;

[kokosing]

Please squash first 3 commits together. Also a good rule is to have cleanup commits before actual change, so it is easier to extract them to separate PR and when you address review comments then it lower chance for conflicts.

[hashhar]

Looks good overall.

Some minor comments.

[hashhar]

Looks good % a couple of nitpicks.

A suggestion about one of the tests.

[hashhar]

LGTM % comments.

@wendigo Should we add a test in BaseJdbcConnectorTest which tries to set the session property and run an insert? Does that actually test anything?

[wendigo]

@hashhar I don't know whether it adds value since we are not able to determine how many batches were executed.

@sergey-melnychuk Maybe let's add insert test with a small and large batch sizes (and insert e.g. 100 rows with a batch size = 1, 50, 64, 100 and 10000) to see how JdbcPageSink handles multiple and single batch inserts.

[sergey-melnychuk]

@hashhar @wendigo WDYT about slightly refactored JdbcPageSink in 2f00609 ? I have extracted factory methods into JdbcPageSinkProvider (the only user of JdbcPageSink ctor) to de-couple extended initialisation logic. This allows introducing simple and convenient way to test if batch size property is respected during page appends.

Such test can be extended to other methods of JdbcPageSink, but IMO it would be out of scope of this PR.

[kokosing]

Skimmed

[sergey-melnychuk]

Thanks @kokosing , will put refactoring and unit-test out to separate PR and then add e2e test similar to existing ones.

IMO such refactoring still introduces SRP and separation of concerns, seems like lack of them makes testing of JdbcPageSink unnecessary complicated. But I will 'disagree and commit' anyway :)

[sergey-melnychuk]

Leaving commit with refactoring out of this PR, now preparing the test using BaseJdbcConnectorTest as a blueprint.

[sergey-melnychuk]

Added simple e2e test that simply inserts n rows with insert_batch_size=k: https://github.com/trinodb/trino/pull/8434/files#diff-6d69520925faa8f876d78d002c7ad94202dfc4a81a9758a5bc3f2ba93b40e023R1296

[hashhar]

LGTM % comment.

Looks good to go otherwise.

[wendigo]

LGTM, please squash commits into single one @sergey-melnychuk

[findepi]

Looking at the code comments (none), it's not clear why we want to have this configurable.

As in #3775 (comment) -- we should understand why this needs to be configurable before making it so.
Didn't read PR comments, but ideally this knowledge should be retained as a code comment.

[sergey-melnychuk]

@findepi PTAL #8527
cc @wendigo

[findepi]

is it applicable to CREATE TABLE AS as well?

[hashhar]

Yes. The PageSink gets used there too. write.batch-size?

[findepi]

sounds good!

[sergey-melnychuk]

@hashhar @findepi @wendigo PTAL #8609

[findepi]

This doesn't belong to JdbcMetadataConfig, since it's not consumed in JdbcMetadata layer

[hashhar]

I think this is mostly because there were no session properties contributed by BaseJdbcConfig.
Maybe this should change? WDYT?

[masaevke]

Hi @hashhar, @findepi,

Thank you for the discussion. I'm currently working with Trino 465 (OSS) and using the JDBC connector to insert data into SQL Server.

I've observed that even after setting write_batch_size = 5000, the inserts are still performed row-by-row in SQL Server Profiler (sp_execute per row), which impacts performance significantly for large datasets.

From my analysis:

• The property insert_batch_size was introduced in PR Add configurable insert_batch_size JDBC session property #8434 to control batch size.
• However, in Open Source Trino, this property is not exposed as a session property (SET SESSION insert_batch_size = ...).
• Instead, write_batch_size is used, but it seems to be ignored or not applied effectively in some cases.

My question:
Is there a known reason why write_batch_size doesn't lead to true batched inserts (e.g., multi-row VALUES or bulk operations) when using the JDBC connector?

I'd appreciate any insights or guidance on how to achieve better batching behavior.

Thanks for your time and contributions to the project!

[Praveen2112]

Hi !! The property was renamed write_batch_size so we could SET SESSION <catalog_name>.write_batch_size-... for configuring it.

[Praveen2112]

write_batch_size is more of a JDBC specific generic property but it doesn't guarantee that writes operations are performed in bulk - it depends on how the JDBC driver is being implemented. In case of SQLServer sp_execute could be invoked per row. Have we tried by setting bulkInsertCopy property in SQLServer - https://learn.microsoft.com/en-us/sql/connect/jdbc/using-bulk-copy-with-the-jdbc-driver?view=sql-server-ver17#sqlserverbulkcopyoptions

[masaevke]

Hi @Praveen2112,

Thank you for the clarification!

I understand that write_batch_size is a JDBC-specific property and doesn't guarantee true bulk inserts — it only controls batch size for parameterized statements.

However, I'm trying to achieve real bulk insert performance (like BULK INSERT or bcp) when inserting 1.5M rows from Trino to MSSQL.

My goal is to see in SQL Server Profiler:

BULK INSERT [trino_test].[aaaaaa].[orders] 
FROM 'virtual_stream' 
WITH (TABLOCK, BATCHSIZE = 1000)

[masaevke]

Could you please advise what configuration changes are required in Trino 465 (Open Source) to enable this behavior?

I've already confirmed that:
dbs-ai-sqldev_trino-test.bulk_copy_for_write = true is set in the catalog
retry_policy = 'NONE' is set in the session
Despite this, I still observe row-by-row inserts via sp_execute in SQL Profiler.

Is there any additional configuration or known limitation in Open Source Trino that prevents the use of SQLServerBulkCopy API?

[findepi]

same here -- it shouldn't be in this class, since it's not consumed by metadata layer