The Field mode normalization for null values is incorrect and sets it as an empty String.

[keskival]

The Field mode normalization for null values is incorrect and sets it as an empty String.

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/Field.java

Lines 199 to 203 in 5703b70

	/** Sets the mode of the field. When not specified {@link Mode#NULLABLE} is used. */
	public Builder setMode(Mode mode) {
	this.mode = mode != null ? mode.name() : Data.<String>nullOf(String.class);
	return this;
	}

Instead of setting the mode String as "" here, it should be set to Field.Mode.NULLABLE.name().

This causes the following problem: When the Field is deserialized from protobuf response from BigQuery, it might have mode set to null. Specifically this seems to happen when a table has been created with SQL CREATE TABLE as it doesn't support setting the field as NULLABLE explicitly.

Steps to reproduce

Create a BigQuery table with some nullable columns using CREATE TABLE.

Get the schema from Java with bigQuery.getTable().getSchema(), compare with a Schema you have created by setting the mode to Field.Mode.NULLABLE explicitly using Field.equals(field2).

Expected result: Both Schemas are equal.

Result: Schemas are inequal. This is because comparison is done for Protobuf representation where an empty String is a placeholder for null.

Code example

  @Test
  public void assertThatSchemaEqualsComparisonIsDeterministic() {
    final Field field1 = Field
      .of("field1", LegacySQLTypeName.INTEGER)
      .toBuilder()
      .setMode(Field.Mode.NULLABLE)
      .build();
    final Field field1b = Field
      .of("field1", LegacySQLTypeName.INTEGER)
      .toBuilder()
      .setMode(null)
      .build();
    final Schema schema1 = Schema.of(field1);
    final Schema schema2 = Schema.of(field1b);
    assertThat(schema1.equals(schema2)).isEqualTo(true);
  }

[keskival]

Alternatively, this should use Data.isNull:

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/Field.java

Line 341 in 5703b70

return mode != null ? Mode.valueOf(mode) : null;

But while that fixes the getter, it doesn't fix the normalization of the default value.

[obada-ab]

Alternatively, this should use Data.isNull:

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/Field.java

Line 341 in 5703b70

return mode != null ? Mode.valueOf(mode) : null;

But while that fixes the getter, it doesn't fix the normalization of the default value.

I think this change makes sense.

And as for normalization, I don't think null field mode needs to be equivalent to Field.Mode.NULLABLE at the client side. They might be treated the same by BigQuery but the client code shouldn't be coupled to this behaviour.