Skip to content

Write struct support#15

Open
stevelordbq wants to merge 6 commits intomainfrom
write-struct-support
Open

Write struct support#15
stevelordbq wants to merge 6 commits intomainfrom
write-struct-support

Conversation

@stevelordbq
Copy link
Copy Markdown

No description provided.

@stevelordbq
Copy link
Copy Markdown
Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for writing Spark Struct and Array[Struct] types to Google Cloud Spanner. A critical flaw was identified in the handling of struct arrays, where the code incorrectly assumes the first element is non-null when determining the element type. This can lead to a NullPointerException and a potential Denial of Service if the input data contains a null as the first element of such an array. Beyond this, there are critical issues regarding the general handling of null and empty values for structs and struct arrays, which could lead to data correctness problems or runtime exceptions. The tests also contain some hardcoded values and incorrect assertions that should be addressed.

{"timestamp", DataTypes.TimestampType, Value.timestamp(null)},
{"date", DataTypes.DateType, Value.date(null)},
{"decimal", new DecimalType(38, 9), Value.numeric(null)},
{"struct", new StructType(), Value.struct(Struct.newBuilder().build())},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

This test asserts that a null struct should be converted to an empty struct. This is inconsistent with how other null types are handled and validates incorrect behavior. A null struct from Spark should result in a NULL value in Spanner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant