Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unecessary limit for OUTPUT command when using batch insert - mssql #440

Closed
MychauU opened this issue Nov 29, 2018 · 6 comments
Closed

unecessary limit for OUTPUT command when using batch insert - mssql #440

MychauU opened this issue Nov 29, 2018 · 6 comments
Labels
sql server waiting for reply Additional information required

Comments

@MychauU
Copy link

MychauU commented Nov 29, 2018

output paramers are only ids. Don't know why even this limit is even implemented.
instead of:
if (paramsToInsert * (data.size + 1) > OUTPUT_PARAMS_LIMIT) {
it should by like:
if ((data.size + 1) > OUTPUT_PARAMS_LIMIT) {
or deleted

Run some tests with parameters(columns ) more than 13 for 1000 rows and it get inserted.

I know that it won't be optimum to insert 1000 rows at once for some cases as it is explained here: https://stackoverflow.com/questions/8635818/multiple-insert-statements-vs-single-insert-with-multiple-values/8640583#8640583
but that it is not the case i write for.

My problem is that some tables have more or less columns and this limit shouldn't even throw exception.

@Tapac
Copy link
Contributor

Tapac commented Nov 30, 2018

Do I understand right that the limitation should be based on (inserted rows * inserted columns) < 1000?
I can't find any information about OUTPUT limitation.
Did you insert 13 * 1000 with Table.batchInsert() syntax or with raw SQLServerBatchInsertStatement call?

@MychauU
Copy link
Author

MychauU commented Nov 30, 2018

I used Table.batchInsert() (but it called later SQLServerBatchInsertStatement as i was using mssql as a database)

I couldn't find any limitation for output command on limit of columns returned, only for select, insert command for column passed = 4096 https://docs.microsoft.com/en-us/sql/sql-server/maximum-capacity-specifications-for-sql-server?view=sql-server-2017

I was trying to insert 1000 rows to table which have 15 columns (i wrote in above comment 13 - my bad). As u can see by not valid if which is in SQLServerBatchInsertStatement the batch insert for this particular example will fail on 334 rows = 334 *15 = 5010

So for me the limitation should be only on maximum number of passed rows in batch insert (as in mssql it is 1000 rows per one insert command (I tested it too) https://social.msdn.microsoft.com/Forums/sqlserver/en-US/bff53b3d-bf50-413f-891e-75af427394e2/limit-to-number-of-insert-statements-or-values-clauses?forum=transactsql)

So the code:
if (paramsToInsert * (data.size + 1) > OUTPUT_PARAMS_LIMIT) { throw BatchDataInconsistentException("Too much parameters for batch with OUTPUT. Exceed $OUTPUT_PARAMS_LIMIT limit") }
should be deleted

@Tapac
Copy link
Contributor

Tapac commented Nov 30, 2018

  1. batchInsert catches BatchDataInconsistentException, executes chunk of a batch (before limitation happens) and then creates new BatchDataInconsistentException, so you'll never see BatchDataInconsistentException while using batchInsert.

  2. AFAIK, SQLServer won't fail if you'll try to execute batch insert with any size of values, but such queries will rather slow compared to multiple smaller batches. The main idea is to split a whole batch to chunks which will be lower than that limit.

I don't use SQLServer and that's why I don't clearly understand the parameter limitation, so, for now, I think that if (paramsToInsert * (data.size + 1) > OUTPUT_PARAMS_LIMIT) should be replaced with if (paramsToInsert * (data.size + 1) > 1000) to prevent slowdowns.

@Tapac Tapac added the waiting for reply Additional information required label Dec 2, 2018
@MychauU
Copy link
Author

MychauU commented Dec 4, 2018

  1. OK, I understand
  2. OK, I understand

I don't use SQLServer and that's why I don't clearly understand the parameter limitation, so, for now, I think that if (paramsToInsert * (data.size + 1) > OUTPUT_PARAMS_LIMIT) should be replaced with if (paramsToInsert * (data.size + 1) > 1000) to prevent slowdowns.

From my perspective to get clean resolution between SQLServer dialect and this particular library there should not be limitation on something that don't exists.
from code:
override fun validateLastBatch() { super.validateLastBatch() if (data.size > OUTPUT_ROW_LIMIT) { throw BatchDataInconsistentException("Too much rows in one batch. Exceed $OUTPUT_ROW_LIMIT limit") } val paramsToInsert = data.firstOrNull()?.size ?: 0 if (paramsToInsert * (data.size + 1) > OUTPUT_PARAMS_LIMIT) { throw BatchDataInconsistentException("Too much parameters for batch with OUTPUT. Exceed $OUTPUT_PARAMS_LIMIT limit") } }
to code:
override fun validateLastBatch() { super.validateLastBatch() if (data.size > OUTPUT_ROW_LIMIT) { throw BatchDataInconsistentException("Too much rows in one batch. Exceed $OUTPUT_ROW_LIMIT limit") } }

@Tapac
Copy link
Contributor

Tapac commented Dec 4, 2018

Does that means what an issue with performance degradation on inserting more than 1000 values in a single query stated here was resolved?

@Tapac
Copy link
Contributor

Tapac commented Aug 25, 2020

Will be removed in 0.27.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql server waiting for reply Additional information required
Projects
None yet
Development

No branches or pull requests

2 participants