Support Java-style blob field directives#490
Conversation
573c68f to
9aa4b4b
Compare
| BLOB_FIELD_DIRECTIVE => BLOB_FIELD_OPTION, | ||
| BLOB_DESCRIPTOR_FIELD_DIRECTIVE => BLOB_DESCRIPTOR_FIELD_OPTION, | ||
| BLOB_VIEW_FIELD_DIRECTIVE => BLOB_VIEW_FIELD_OPTION, | ||
| _ => return None, |
There was a problem hiding this comment.
This should not silently fall back to a normal comment when the column comment starts with __BLOB but does not match one of the supported directives.
Java Paimon rejects unknown blob directives in this case, e.g. __BLOB_UNKNOWN, so a typo like __BLOB_FIEL; picture should fail fast instead of creating a plain binary column. Otherwise the user gets a non-BLOB schema with no blob-field option and only discovers the issue later.
Could we make this parser return an error for unknown __BLOB* directives and add a create/add-column test for that case?
There was a problem hiding this comment.
Add test_blob_comment_directive_rejects_unknown_create_directive.
1f65ee5 to
93a5e09
Compare
leaves12138
left a comment
There was a problem hiding this comment.
One small Java-compatibility gap remains in the directive parser.
Java ColumnDirectiveUtils.matchDirective accepts only a marker by itself or a marker immediately followed by ;. For example, __BLOB_FIELD; comment is valid, but __BLOB_FIELD ; comment is rejected because the character after __BLOB_FIELD is a space, not ;.
The Rust parser currently does splitn(2, ';') and then trims the directive part, so COMMENT '__BLOB_FIELD ; payload bytes' is accepted, stripped, and converted into a BLOB column. I verified this locally with a temporary regression test: Schema::builder() returned Ok with blob-field=payload, whereas Java would throw an unsupported directive error.
Could we align the matching with Java by trimming the whole comment first and accepting only marker or marker;... (no whitespace between the marker and semicolon), and add a regression test for this malformed directive?
93a5e09 to
e10cfa9
Compare
leaves12138
left a comment
There was a problem hiding this comment.
Looks good to me. The Java-style BLOB directive parsing and validation issues from the previous review are addressed.
Summary
Adds Java-compatible BLOB field comment directives so SQL schemas can mark binary columns as raw BLOB, descriptor BLOB, or Blob View fields. The directive handling lives in the core schema layer, while DataFusion only passes through column comments and supports the
BYTESSQL alias.Changes
__BLOB_FIELD,__BLOB_DESCRIPTOR_FIELD, and__BLOB_VIEW_FIELDduring core schema creation andALTER TABLE ... ADD COLUMN.BlobType, updateblob-field/blob-descriptor-field/blob-view-fieldoptions, and strip directive prefixes from stored comments.BYTESand Java-style BLOB directives.docs/src/sql.md.Testing
cargo test -p paimon blob_comment_directive --libcargo test -p paimon blob_field_option --libcargo test -p paimon test_blob_view_options --libcargo test -p paimon-datafusion blob_comment_directive --libcargo test -p paimon-datafusion test_create_table_blob --libcargo test -p paimon-datafusion test_alter_table_add_blob --libgit diff --check