Skip to content

GCP, Azure: ignore null values for optional numeric properties - #17784

Open
waterWang wants to merge 1 commit into
apache:mainfrom
waterWang:fix/null-safe-property-parsing
Open

GCP, Azure: ignore null values for optional numeric properties#17784
waterWang wants to merge 1 commit into
apache:mainfrom
waterWang:fix/null-safe-property-parsing

Conversation

@waterWang

Copy link
Copy Markdown

Problem

GCPProperties and AzureProperties throw NumberFormatException when optional numeric properties exist in the properties map with null values.

properties.containsKey(...) returns true for null-valued entries, but Integer.parseInt(null) / Long.parseLong(null) raises NumberFormatException: Cannot parse null string.

Changes

  • GCPProperties.java: Replaced Integer.parseInt(properties.get(...)) with PropertyUtil.propertyAsNullableInt for GCS_CHANNEL_READ_CHUNK_SIZE and GCS_CHANNEL_WRITE_CHUNK_SIZE. Replaced Long.parseLong(properties.get(...)) with PropertyUtil.propertyAsNullableLong for GCS_OAUTH2_TOKEN_EXPIRES_AT, with a null guard before the new Date(...) call.

  • AzureProperties.java: Replaced Integer.parseInt(properties.get(...)) with PropertyUtil.propertyAsNullableInt for ADLS_READ_BLOCK_SIZE. Replaced Long.parseLong(properties.get(...)) with PropertyUtil.propertyAsNullableLong for ADLS_WRITE_BLOCK_SIZE.

  • TestGCPProperties.java: Added testNullNumericPropertiesAreIgnored to verify null values are treated as absent.

  • TestAzureProperties.java: Added testNullBlockSizesAreIgnored to verify null values are treated as absent.

Fixes #17771

Comment on lines +122 to +123
this.adlsReadBlockSize =
PropertyUtil.propertyAsNullableInt(properties, ADLS_READ_BLOCK_SIZE);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can eliminate the if condition above, right?

}

@Test
public void testNullBlockSizesAreIgnored() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

https://iceberg.apache.org/contribute/#conventions-and-recommendations

Omit the test prefix for newly added test methods.

Suggested change
public void testNullBlockSizesAreIgnored() {
public void nullValuesAreIgnored() {

There is no need to include "BlockSizes" in my opinion. We may add new entries in the future.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: GCPProperties and AzureProperties throw NumberFormatException on null property values

2 participants