Skip to content
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
b727aa0
Use Bulk Copy API for batch insert operation
peterbae Apr 20, 2018
52da9aa
Parse bug fixing and test added
peterbae Apr 27, 2018
5bb79a2
bug fix + additional tests
peterbae May 1, 2018
5cf28ad
change reflection for testing
peterbae May 1, 2018
59d29d7
more test changes
peterbae May 1, 2018
dc42708
Add parsing logic for -- comment
peterbae May 1, 2018
f67cad1
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae May 3, 2018
dca4cb5
refactoring
peterbae May 3, 2018
1c6a186
Merge branch 'dev' into batch-insert-improvement
cheenamalhotra May 3, 2018
80112c5
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae May 7, 2018
8ed14fe
Merge branch 'batch-insert-improvement' of https://github.com/peterba…
peterbae May 7, 2018
b67ccfb
Bug fix / testing change
peterbae May 8, 2018
a677b28
Reflect comment change
peterbae May 8, 2018
60b437e
Refactor two Bulk files into a common parent
peterbae May 25, 2018
8604ff4
javadoc changes
peterbae May 25, 2018
34d8bb1
fix problem with precision / scale
peterbae May 28, 2018
39060b7
fix issue with setting all to true
peterbae May 29, 2018
d80908e
make bamoo fixes
peterbae May 29, 2018
aa34276
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae May 30, 2018
8d044b1
undo some changes made to SQLServerConnection
peterbae May 30, 2018
70bea50
apply resource bundling changes
peterbae May 30, 2018
8b45a21
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae Jun 4, 2018
c204354
remove on_dw, and remove redundant fmtonly
peterbae Jun 5, 2018
f218fe3
formatting
peterbae Jun 5, 2018
3c5d023
Reformatting + adding more tests
peterbae Jun 6, 2018
dcd63d3
inherit the connection property in statement + fix issue with null / …
peterbae Jun 8, 2018
c322174
comment revisions
peterbae Jun 13, 2018
0700749
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae Jun 14, 2018
7dd2d42
Fixing logic / adding more tests
peterbae Jun 19, 2018
84acbb8
dont use test database in tests
peterbae Jun 19, 2018
38bcdc6
Change exception handling as per JDBC specs
peterbae Jun 20, 2018
b4d4b26
remove some comments
peterbae Jun 20, 2018
9b7f40c
reflect comments
peterbae Jun 22, 2018
c6f8011
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae Jun 22, 2018
405b47e
changed how logger works, refactored code in SQLServerBulkCommon due …
peterbae Jun 22, 2018
66c0b19
add more tests, make the prepared statement property go away
peterbae Jun 22, 2018
0b65647
Merge branch 'dev' into batch-insert-improvement
cheenamalhotra Jun 24, 2018
80b396a
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into ba…
peterbae Jun 25, 2018
d2d5d23
Merge branch 'batch-insert-improvement' of https://github.com/peterba…
peterbae Jun 25, 2018
2369bb2
Added getter/setter public for the useBulkCopyForBatchInsert connecti…
peterbae Jun 25, 2018
bae637c
Change implementation of child classes a bit
peterbae Jun 25, 2018
b8afb58
Fix bamboo problem + refactor test code
peterbae Jun 26, 2018
096d78e
Replace all connection and statements with try blocks
peterbae Jun 26, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/main/java/com/microsoft/sqlserver/jdbc/Parameter.java
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,10 @@ Object getValue(JDBCType jdbcType,
// statement level), cryptoMeta would be null.
return getterDTV.getValue(jdbcType, outScale, getterArgs, cal, typeInfo, cryptoMeta, tdsReader);
}

Object getSetterValue() {
return setterDTV.getSetterValue();
}

int getInt(TDSReader tdsReader) throws SQLServerException {
Integer value = (Integer) getValue(JDBCType.INTEGER, null, null, tdsReader);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,322 @@
/*
* Microsoft JDBC Driver for SQL Server
*
* Copyright(c) Microsoft Corporation All rights reserved.
*
* This program is made available under the terms of the MIT License. See the LICENSE file in the project root for more information.
*/

package com.microsoft.sqlserver.jdbc;

import java.math.BigDecimal;
import java.math.RoundingMode;
import java.sql.Types;
import java.text.DecimalFormat;
import java.text.MessageFormat;
import java.time.OffsetDateTime;
import java.time.OffsetTime;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map.Entry;
import java.util.Set;

/**
* A simple implementation of the ISQLServerBulkRecord interface that can be used to read in the basic Java data types from an ArrayList of Parameters
* that were provided by pstmt/cstmt.
*/
public class SQLServerBulkBatchInsertRecord extends SQLServerBulkCommon implements ISQLServerBulkRecord, java.lang.AutoCloseable {

private ArrayList<Parameter[]> batchParam;
private int batchParamIndex = -1;
private ArrayList<String> columnList;
private ArrayList<String> valueList;

public SQLServerBulkBatchInsertRecord(ArrayList<Parameter[]> batchParam,
ArrayList<String> columnList,
ArrayList<String> valueList,
String encoding) throws SQLServerException {
loggerClassName = "com.microsoft.sqlserver.jdbc.SQLServerBulkBatchInsertRecord";
loggerExternal = java.util.logging.Logger.getLogger(loggerClassName);
loggerExternal.entering(loggerClassName, "SQLServerBulkBatchInsertRecord", new Object[] {batchParam, encoding});

if (null == batchParam) {
throwInvalidArgument("batchParam");
}

if (null == valueList) {
throwInvalidArgument("valueList");
}

this.batchParam = batchParam;
this.columnList = columnList;
this.valueList = valueList;
columnMetadata = new HashMap<>();

loggerExternal.exiting(loggerClassName, "SQLServerBulkBatchInsertRecord");
}

/**
* Releases any resources associated with the batch.
*
* @throws SQLServerException
* when an error occurs
*/
public void close() throws SQLServerException {
}

public DateTimeFormatter getColumnDateTimeFormatter(int column) {
return columnMetadata.get(column).dateTimeFormatter;
}

@Override
public Set<Integer> getColumnOrdinals() {
return columnMetadata.keySet();
}

@Override
public String getColumnName(int column) {
return columnMetadata.get(column).columnName;
}

@Override
public int getColumnType(int column) {
return columnMetadata.get(column).columnType;
}

@Override
public int getPrecision(int column) {
return columnMetadata.get(column).precision;
}

@Override
public int getScale(int column) {
return columnMetadata.get(column).scale;
}

@Override
public boolean isAutoIncrement(int column) {
return false;
}

private Object convertValue(ColumnMetadata cm,
Object data) throws SQLServerException {
switch (cm.columnType) {
/*
* Both BCP and BULK INSERT considers double quotes as part of the data and throws error if any data (say "10") is to be inserted into an
* numeric column. Our implementation does the same.
*/
case Types.INTEGER: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#531
#538

Do we have to convert everything to String?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to. I'm following the implementation of bulk copy from SQLServerBulkCSVFileRecord. The PRs you linked are not related with these files - we can look into optimizing the bulk copy process in another PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out we also need to do this, actually. The alternative of using instanceof would be messy here, since the user can provide any type of numeric value through setObject (they could do 123.45f or 12345L to instantiate those numbers as float, double or long, etc), and we would need an if statement for each of those types.

That being said, I found an issue with the existing implementation - it was actually not flooring the data. The number 123.56 would be inserted as 124 into the column, whereas it should've been 123. I've fixed this issue.

@ulvii ulvii Jun 14, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we have to or can fix these, but a few things to note here:

  1. We convert everything to String first, then to Double and back to Integer. @cheenamalhotra might have some insights, since she was working on Code improvements in SQLServerDataTable > 'internalAddRow()' #538.
  2. If user sets a value that doesn't fit into an Integer using setObject(), the new implementation throws SQLServerException, whereas normal batch update would throw BatchUpdateException. There might be other scenarios that I haven't looked into.

@cheenamalhotra , Does the new property have to be transparent to the applications in regards to batch inserts?

// Formatter to remove the decimal part as SQL Server floors the decimal in integer types
DecimalFormat decimalFormatter = new DecimalFormat("#");
decimalFormatter.setRoundingMode(RoundingMode.DOWN);
String formatedfInput = decimalFormatter.format(Double.parseDouble(data.toString()));
return Integer.valueOf(formatedfInput);
}

case Types.TINYINT:
case Types.SMALLINT: {
// Formatter to remove the decimal part as SQL Server floors the decimal in integer types
DecimalFormat decimalFormatter = new DecimalFormat("#");
decimalFormatter.setRoundingMode(RoundingMode.DOWN);
String formatedfInput = decimalFormatter.format(Double.parseDouble(data.toString()));
return Short.valueOf(formatedfInput);
}

case Types.BIGINT: {
BigDecimal bd = new BigDecimal(data.toString().trim());
try {
return bd.setScale(0, RoundingMode.DOWN).longValueExact();
}
catch (ArithmeticException ex) {
String value = "'" + data + "'";
MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_errorConvertingValue"));
throw new SQLServerException(form.format(new Object[] {value, JDBCType.of(cm.columnType)}), null, 0, ex);
}
}

case Types.DECIMAL:
case Types.NUMERIC: {
BigDecimal bd = new BigDecimal(data.toString().trim());
return bd.setScale(cm.scale, RoundingMode.HALF_UP);
}

case Types.BIT: {
// "true" => 1, "false" => 0
// Any non-zero value (integer/double) => 1, 0/0.0 => 0
try {
return (0 == Double.parseDouble(data.toString())) ? Boolean.FALSE : Boolean.TRUE;
}
catch (NumberFormatException e) {
return Boolean.parseBoolean(data.toString());
}
}

case Types.REAL: {
return Float.parseFloat(data.toString());
}

case Types.DOUBLE: {
return Double.parseDouble(data.toString());
}

case Types.BINARY:
case Types.VARBINARY:
case Types.LONGVARBINARY:
case Types.BLOB: {
// Strip off 0x if present.
String binData = data.toString().trim();
if (binData.startsWith("0x") || binData.startsWith("0X")) {
return binData.substring(2);
}
else {
return binData;
}
}

case 2013: // java.sql.Types.TIME_WITH_TIMEZONE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we have to use hardcoded 2013 instead of Types. TIME_WITH_TIMEZONE here? same with 2014 and TIMESTAMP_WITH_TIMEZONE?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code for java.sql.Types, looks like TIME_WITH_TIMEZONE and TIMESTAMP_WITH_TIMEZONE was added later on in JDBC 4.2. They probably wrote this code before those values were added to the Types file, but knew the number code beforehand. I've made the changes to use the actual name.

{
OffsetTime offsetTimeValue;

// The per-column DateTimeFormatter gets priority.
if (null != cm.dateTimeFormatter)
offsetTimeValue = OffsetTime.parse(data.toString(), cm.dateTimeFormatter);
else if (timeFormatter != null)
offsetTimeValue = OffsetTime.parse(data.toString(), timeFormatter);
else
offsetTimeValue = OffsetTime.parse(data.toString());

return offsetTimeValue;
}

case 2014: // java.sql.Types.TIMESTAMP_WITH_TIMEZONE
{
OffsetDateTime offsetDateTimeValue;

// The per-column DateTimeFormatter gets priority.
if (null != cm.dateTimeFormatter)
offsetDateTimeValue = OffsetDateTime.parse(data.toString(), cm.dateTimeFormatter);
else if (dateTimeFormatter != null)
offsetDateTimeValue = OffsetDateTime.parse(data.toString(), dateTimeFormatter);
else
offsetDateTimeValue = OffsetDateTime.parse(data.toString());

return offsetDateTimeValue;
}

case Types.NULL: {
return null;
}

case Types.DATE:
case Types.CHAR:
case Types.NCHAR:
case Types.VARCHAR:
case Types.NVARCHAR:
case Types.LONGVARCHAR:
case Types.LONGNVARCHAR:
case Types.CLOB:
default: {
// The string is copied as is.
return data;
}
}
}

private String removeSingleQuote(String s) {
int len = s.length();
return (s.charAt(0) == '\'' && s.charAt(len - 1) == '\'') ? s.substring(1, len - 1) : s;
}

@Override
public Object[] getRowData() throws SQLServerException {
Object[] data = new Object[columnMetadata.size()];
int valueIndex = 0;
String valueData;
Object rowData;
int columnListIndex = 0;

// check if the size of the list of values = size of the list of columns (which is optional)
if (null != columnList && columnList.size() != valueList.size()) {
MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_DataSchemaMismatch"));
Object[] msgArgs = {};
throw new SQLServerException(form.format(msgArgs), SQLState.COL_NOT_FOUND, DriverError.NOT_SET, null);
}

for (Entry<Integer, ColumnMetadata> pair : columnMetadata.entrySet()) {
int index = pair.getKey() - 1;

// To explain what each variable represents:
// columnMetadata = map containing the ENTIRE list of columns in the table.
// columnList = the *optional* list of columns the user can provide. For example, the (c1, c3) part of this query: INSERT into t1 (c1, c3) values (?, ?)
// valueList = the *mandatory* list of columns the user needs provide. This is the (?, ?) part of the previous query. The size of this valueList will always equal the number of
// the entire columns in the table IF columnList has NOT been provided. If columnList HAS been provided, then this valueList may be smaller than the list of all columns (which is columnMetadata).

// case when the user has not provided the optional list of column names.
if (null == columnList || columnList.size() == 0) {
valueData = valueList.get(index);
// if the user has provided a wildcard for this column, fetch the set value from the batchParam.
if (valueData.equalsIgnoreCase("?")) {
rowData = batchParam.get(batchParamIndex)[valueIndex++].getSetterValue();
}
else if (valueData.equalsIgnoreCase("null")) {
rowData = null;
}
// if the user has provided a hardcoded value for this column, rowData is simply set to the hardcoded value.
else {
rowData = removeSingleQuote(valueData);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test insert into t values ('''')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works. I've added this scenario as a test case as well.

}
}
// case when the user has provided the optional list of column names.
else {
// columnListIndex is a separate counter we need to keep track of for each time we've processed a column
// that the user provided.
// for example, if the user provided an optional columnList of (c1, c3, c5, c7) in a table that has 8 columns (c1~c8),
// then the columnListIndex would increment only when we're dealing with the four columns inside columnMetadata.
// compare the list of the optional list of column names to the table's metadata, and match each other, so we assign the correct value to each column.
if (columnList.size() > columnListIndex && columnList.get(columnListIndex).equalsIgnoreCase(columnMetadata.get(index + 1).columnName)) {
valueData = valueList.get(columnListIndex);
if (valueData.equalsIgnoreCase("?")) {
rowData = batchParam.get(batchParamIndex)[valueIndex++].getSetterValue();
}
else if (valueData.equalsIgnoreCase("null")) {
rowData = null;
}
else {
rowData = removeSingleQuote(valueData);
}
columnListIndex++;
}
else {
rowData = null;
}
}

try {
if (null == rowData) {
data[index] = null;
continue;
} else if (0 == rowData.toString().length()) {
data[index] = "";
continue;
}
data[index] = convertValue(pair.getValue(), rowData);
}
catch (IllegalArgumentException e) {
String value = "'" + rowData + "'";
MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_errorConvertingValue"));
throw new SQLServerException(form.format(new Object[] {value, JDBCType.of(pair.getValue().columnType)}), null, 0, e);
}
catch (ArrayIndexOutOfBoundsException e) {
throw new SQLServerException(SQLServerException.getErrString("R_DataSchemaMismatch"), e);
}
}
return data;
}

@Override
public boolean next() throws SQLServerException {
batchParamIndex++;
return batchParamIndex < batchParam.size();
}
}
Loading