Repository navigation
Conversation
Stream and unwrap BigQuery v2 REST TableCell 'v' wrappers directly in HttpBigQueryRpc for listTableData, getQueryResults, and queryRpc responses instead of reflectively instantiating GenericJson TableCell and ArrayMap objects per cell.
There was a problem hiding this comment.
Code Review
This pull request optimizes BigQuery REST response parsing by implementing a custom streaming JSON parser in HttpBigQueryRpc to bypass reflective TableCell allocations, significantly reducing memory overhead. It also updates FieldValue to support unwrapped cell representations and adds corresponding unit tests. The code review highlights critical robustness issues in the new parser: several loops parsing JSON arrays do not check for EOF (null tokens), which could cause infinite loops and OutOfMemoryError on truncated or malformed streams. Additionally, a potential UnsupportedOperationException was identified when attempting to mutate an immutable singleton map if unexpected extra keys are encountered after an 'f' field.
| while (parser.nextToken() != JsonToken.END_ARRAY) { | ||
| if (parser.getCurrentToken() != JsonToken.START_OBJECT) { | ||
| parser.skipChildren(); | ||
| continue; | ||
| } | ||
| List<Object> cells = null; | ||
| while (parser.nextToken() == JsonToken.FIELD_NAME) { | ||
| String rowField = parser.getText(); | ||
| parser.nextToken(); | ||
| if ("f".equals(rowField) && parser.getCurrentToken() == JsonToken.START_ARRAY) { | ||
| // Pre-size each row's cell list from the previous row's column count to avoid | ||
| // ArrayList resizing across rows of the same table/query schema. | ||
| cells = new ArrayList<>(expectedCols); | ||
| while (parser.nextToken() != JsonToken.END_ARRAY) { | ||
| cells.add(parseCellValue(parser)); | ||
| } | ||
| expectedCols = cells.size(); | ||
| } else { | ||
| parser.skipChildren(); | ||
| } | ||
| } | ||
| TableRow row = new TableRow(); | ||
| if (cells != null) { | ||
| row.setF((List) cells); | ||
| } | ||
| rows.add(row); | ||
| } |
There was a problem hiding this comment.
If the JSON stream is truncated or malformed, parser.nextToken() can return null (representing EOF). Since null != JsonToken.END_ARRAY is true, the loop will continue. Inside the loop, parser.getCurrentToken() will be null, which is not START_OBJECT, so it calls parser.skipChildren(). Since skipChildren() does nothing when the current token is null, this results in an infinite loop consuming 100% CPU. Additionally, the inner loop reading cells will also loop infinitely adding JSON_NULL to the list, leading to an OutOfMemoryError.
To prevent this, ensure all while (parser.nextToken() != JsonToken.END_ARRAY) loops also check for null (EOF).
JsonToken token;
while ((token = parser.nextToken()) != JsonToken.END_ARRAY && token != null) {
if (token != JsonToken.START_OBJECT) {
parser.skipChildren();
continue;
}
List<Object> cells = null;
while (parser.nextToken() == JsonToken.FIELD_NAME) {
String rowField = parser.getText();
JsonToken valueToken = parser.nextToken();
if ("f".equals(rowField) && valueToken == JsonToken.START_ARRAY) {
// Pre-size each row's cell list from the previous row's column count to avoid
// ArrayList resizing across rows of the same table/query schema.
cells = new ArrayList<>(expectedCols);
JsonToken cellToken;
while ((cellToken = parser.nextToken()) != JsonToken.END_ARRAY && cellToken != null) {
cells.add(parseCellValue(parser));
}
expectedCols = cells.size();
} else {
parser.skipChildren();
}
}
TableRow row = new TableRow();
if (cells != null) {
row.setF((List) cells);
}
rows.add(row);
}| List<Object> subFields = new ArrayList<>(); | ||
| if (parser.getCurrentToken() == JsonToken.START_ARRAY) { | ||
| while (parser.nextToken() != JsonToken.END_ARRAY) { | ||
| subFields.add(parseCellValue(parser)); |
There was a problem hiding this comment.
If the JSON stream is truncated or malformed, parser.nextToken() can return null (representing EOF). Since null != JsonToken.END_ARRAY is true, the loop will continue infinitely, adding JSON_NULL to subFields and eventually causing an OutOfMemoryError.
To prevent this, ensure the loop condition also checks for null (EOF).
| List<Object> subFields = new ArrayList<>(); | |
| if (parser.getCurrentToken() == JsonToken.START_ARRAY) { | |
| while (parser.nextToken() != JsonToken.END_ARRAY) { | |
| subFields.add(parseCellValue(parser)); | |
| if (parser.getCurrentToken() == JsonToken.START_ARRAY) { | |
| JsonToken subToken; | |
| while ((subToken = parser.nextToken()) != JsonToken.END_ARRAY && subToken != null) { | |
| subFields.add(parseCellValue(parser)); | |
| } |
| if (token == JsonToken.START_ARRAY) { | ||
| List<Object> list = new ArrayList<>(); | ||
| while (parser.nextToken() != JsonToken.END_ARRAY) { | ||
| list.add(parseCellValue(parser)); | ||
| } | ||
| return list; | ||
| } |
There was a problem hiding this comment.
If the JSON stream is truncated or malformed, parser.nextToken() can return null (representing EOF). Since null != JsonToken.END_ARRAY is true, the loop will continue infinitely, adding JSON_NULL to the list and eventually causing an OutOfMemoryError.
To prevent this, ensure the loop condition also checks for null (EOF).
| if (token == JsonToken.START_ARRAY) { | |
| List<Object> list = new ArrayList<>(); | |
| while (parser.nextToken() != JsonToken.END_ARRAY) { | |
| list.add(parseCellValue(parser)); | |
| } | |
| return list; | |
| } | |
| if (token == JsonToken.START_ARRAY) { | |
| List<Object> list = new ArrayList<>(); | |
| JsonToken arrayToken; | |
| while ((arrayToken = parser.nextToken()) != JsonToken.END_ARRAY && arrayToken != null) { | |
| list.add(parseCellValue(parser)); | |
| } | |
| return list; | |
| } |
| if (mapValue == null) { | ||
| mapValue = new HashMap<>(); | ||
| if (hasValue) { | ||
| mapValue.put("v", unwrappedValue); | ||
| hasValue = false; | ||
| } | ||
| } | ||
| mapValue.put(key, parseCellValue(parser)); |
There was a problem hiding this comment.
If the JSON object contains an 'f' field followed by unexpected extra keys, mapValue is initialized as an immutable singleton map via Collections.singletonMap("f", subFields). In the next iteration, attempting to call mapValue.put(key, ...) will throw an UnsupportedOperationException.
To prevent this, copy the map to a mutable HashMap if it is not already a HashMap before adding extra keys.
if (mapValue == null) {
mapValue = new HashMap<>();
if (hasValue) {
mapValue.put("v", unwrappedValue);
hasValue = false;
}
} else if (!(mapValue instanceof HashMap)) {
mapValue = new HashMap<>(mapValue);
}
mapValue.put(key, parseCellValue(parser));
Summary
Eliminates reflective
TableCellandArrayMapallocations when parsing row responses inHttpBigQueryRpc(listTableData,listTableDataWithRowLimit,getQueryResults,getQueryResultsWithRowLimit, andqueryRpc).Background
In the BigQuery v2 REST API, each row is serialized as
{"f": [{"v": ...}, {"v": ...}, ...]}. Previously,request.execute()usedgoogle-http-client's reflectiveJsonParserto instantiate aTableCell(extends GenericJson -> GenericData) for every cell:Data.newMapInstance->Types.newInstance(TableCell.class)->Class.getDeclaredConstructor()(allocating aClass[0]array), plusEnumSet.noneOf(...), anArrayMapforunknownFields, and theTableCellinstance itself (4 object allocations per cell).FieldValueList.fromPbandFieldValue.fromPbunwrappedcell.getV()and discarded theTableCell.TableCellimplementsMap<String, Object>,FieldValue.fromPbalso fell intocellPb instanceof Map, triggering reflectiveClassInfo/FieldInfolookups on"f"and"v"for every cell before recursing oncell.getV().Changes
HttpBigQueryRpc.java:executeAndParseRowResponse,parseRowResponse,parseTableRows, andparseCellValueto stream the"rows"array and unwrap single-key{"v": ...}cell objects directly into their inner representations (Stringfor scalars,Data.nullOf(Object.class)fornull,List<Object>forREPEATEDarrays, andCollections.singletonMap("f", subFields)forRECORDstructs)."rows"top-level fields (schema,pageToken,@JsonString64-bit numbers such astotalRowsandtotalBytesProcessed,jobReference,errors, etc.) toClassInfoandJsonParserfor full metadata fidelity.FieldValue.java:cellPb instanceof TableCellfast-path beforecellPb instanceof Mapand checkscellPb == null || cellPb == NULL_CELL_SENTINELbeforeData.isNull(cellPb).