Fix nullable bug of Arrow MapVector in Bridge.cpp

[whutjs]

There is a constraint defined in arrow/format/Schema.fbs, which requires the MapVector itself and the key of MapVector to be not nullable.

/// Neither the "entries" field nor the "key" field may be nullable.

There are also two not-nullable constraints in the Java implementation of MapVector:

1. Map data should be a non-nullable struct type;
   https://github.com/apache/arrow/blob/d4516c5386f84619dfdf2a9f72fed6d7df89704c/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java#L98
2. Map data key type should be a non-nullable;
   https://github.com/apache/arrow/blob/d4516c5386f84619dfdf2a9f72fed6d7df89704c/java/vector/src/main/java/org/apache/arrow/vector/complex/MapVector.java#L105

However, When the velox convert the MapVector to Arrow MapVector, velox will set the flag of MapVector to nullable, which violates these constraints:

velox/velox/vector/arrow/Bridge.cpp

Line 1388 in acd5717

arrowSchema.flags = ARROW_FLAG_NULLABLE;

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	ae1f99a
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/6715c143f3675000087cb26b

[Yuhta]

Shall we add a test for this as well?

[whutjs]

@Yuhta thanks for your review. I add some assertions in the ArrowBridgeSchemaTest to verify that the flag of MapVector should not be nullable.

[whutjs]

@mbasmanova Would you please take a look at this? Thanks.

[mbasmanova]

@whutjs Thank you for the fix. Would you update the PR description to clarify which constraint was violated and, if possible, provide a reference to Arrow documentation that describes that constraint.

[mbasmanova]

Would it make sense to change clearArrowNullableFlag take flags as in/out parameter?

clearArrowNullableFlag(child->flags);

Would it also make sense to remove 'Arrow' from the function name? It seems to be implied.

[whutjs]

@mbasmanova thanks for your review. I have modified this PR according to your suggestion.

[mbasmanova]

Thanks.

[facebook-github-bot]

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[kgpai]

@whutjs Can you rebase against latest main and update ?

[whutjs]

@kgpai DONE