Skip to content

spanner: Unable to insert multiple rows in a single table.insert/transaction.insert call when some rows lack a value for nullable columns #2411

@jscinoz

Description

@jscinoz

In transaction-request.js, there is the following (L687-695):

mutation[method] = {
  table: table,
  columns: Object.keys(keyVals[0]),
  values: keyVals.map(function(keyVal) {
    return Object.keys(keyVal).map(function(key) {
      return codec.encode(keyVal[key]);
    });
  })
};

There are at least two bugs here:

  1. When an array is provided to table.insert(), if some objects do not have a given key (which is perfectly valid if the column said key corresponds to is nullable), the array within values for this object will not be the same length as the columns array, and indices in columns will no longer correspond to the correct value in the values array for this row/object. This results in (best case) misleading error messages from Spanner about incorrect data types. Worse still, if it so happens that the value in the values array for this row and incorrect column index is the same type as the value for the correct column index, the insert request will succeed, and end up storing data in the wrong column entirely.

  2. If a consumer of this library works around the issue above by populating missing nullable keys with null, as the code above relies on the order of values returned by Object.keys, the newly added values will be at the end of the array returned by Object.keys (which returns keys in insertion order), and again, the indices between the columns array, and the values array for affected rows will no longer align, with the same end result as what occurs with issue 1 above.

This could be fixed by iterating over all rows (rather than naively assuming all rows have identical keys to the first row) and collecting all unique keys into a single array which is subsequently sorted lexically, to build the value for columns. Likewise, the value for values would be constructed by mapping over columns (rather than Object.keys(keyVal)) and pulling out the corresponding value from keyVal to pass to codec.encode.

Metadata

Metadata

Labels

api: spannerIssues related to the Spanner API.priority: p0Highest priority. Critical issue. P0 implies highest priority.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions