FANNG1 commented on code in PR #13390:
URL: https://github.com/apache/gravitino/pull/13390#discussion_r4081844144
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceDataTypeConverter.java:
##########
@@ -203,7 +205,29 @@ public ArrowType fromGravitino(Type type) {
@Override
public Type toGravitino(Field arrowField) {
+ Optional<String> blobType = LanceBlobTypes.toCatalogString(arrowField);
+ if (blobType.isPresent()) {
+ return Types.ExternalType.of(blobType.get());
+ }
+
+ // Only Lance blob metadata is recognized. A blob field outside the
canonical layout keeps the
+ // whole field as Arrow JSON so the blob is not turned into a plain binary
or struct column.
+ if (LanceBlobTypes.isBlob(arrowField)) {
+ return toExternalType(arrowField);
+ }
+
FieldType fieldType = arrowField.getFieldType();
+ // List, map and union children are rebuilt with fixed names and without
metadata, so a blob
+ // below them can only be preserved by keeping the whole subtree as Arrow
JSON. Struct children
+ // keep their names and are converted recursively.
+ ArrowType.ArrowTypeID typeId = fieldType.getType().getTypeID();
+ if ((typeId == ArrowType.ArrowTypeID.List
+ || typeId == ArrowType.ArrowTypeID.Map
+ || typeId == ArrowType.ArrowTypeID.Union)
+ &&
arrowField.getChildren().stream().anyMatch(LanceDataTypeConverter::hasBlobInTree))
{
+ return toExternalType(arrowField);
+ }
Review Comment:
Outdated: the List/Map/Union guard was removed in a086e3c7, containers now
stay native. `LargeList` and `FixedSizeList` have no native mapping, so the
whole field already falls through to Arrow JSON and round-trips losslessly;
added tests for both with a blob child.
##########
docs/lakehouse-generic-lance-table.md:
##########
@@ -97,6 +99,37 @@ For Arrow types not natively mapped in Gravitino, use the
`External(arrow_field_
| `Large List` |
`External("{\"name\":\"col_name\",\"nullable\":true,\"type\":{\"name\":\"largelist\"},\"children\":[{\"name\":\"element\",\"nullable\":true,\"type\":{\"name\":\"int\",\"bitWidth\":32,\"isSigned\":true},\"children\":[]}]}")`
|
| `Fixed-Size List` |
`External("{\"name\":\"col_name\",\"nullable\":true,\"type\":{\"name\":\"fixedsizelist\",\"listSize\":10},\"children\":[{\"name\":\"element\",\"nullable\":true,\"type\":{\"name\":\"int\",\"bitWidth\":32,\"isSigned\":true},\"children\":[]}]}")`
|
+Gravitino types cannot carry Arrow field metadata. When loading a Lance table,
only Lance blob metadata
+is recognized (see [Blob Types](#blob-types)); other field metadata is
ignored. If a blob field appears
+anywhere inside a `List`, `Map` or `Union` field, the whole field is returned
as
+`External(arrow_field_json_str)`.
Review Comment:
Outdated: this paragraph was rewritten in a086e3c7. Large/Fixed-Size List
already map to `External(arrow_field_json_str)` per the type table.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceBlobTypes.java:
##########
@@ -0,0 +1,316 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.lance.common.ops.gravitino;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+
+/**
+ * Converts Lance blob columns between their Arrow field form and a readable
catalog string used by
+ * Gravitino external types.
+ *
+ * <p>Supported catalog strings:
+ *
+ * <ul>
+ * <li>{@code lance.blob.v1}: an Arrow {@code LargeBinary} field with
metadata {@code
+ * lance-encoding:blob=true}.
+ * <li>{@code lance.blob.v2(with_range=true, inline_size_threshold=N,
dedicated_size_threshold=N,
+ * pack_file_size_threshold=N)}: an Arrow struct tagged with {@code
+ * ARROW:extension:name=lance.blob.v2}. All parameters are optional;
without parameters the
+ * parentheses are omitted.
+ * </ul>
+ *
+ * <p>Only Lance blob metadata is recognized; other field metadata is not
represented. A blob field
+ * that does not exactly match the canonical Lance layout is left to the Arrow
JSON representation.
+ */
+final class LanceBlobTypes {
+
+ static final String BLOB_META_KEY = "lance-encoding:blob";
+ static final String ARROW_EXT_NAME_KEY = "ARROW:extension:name";
+ static final String BLOB_V2_EXT_NAME = "lance.blob.v2";
+ static final String INLINE_SIZE_THRESHOLD_META_KEY =
"lance-encoding:blob-inline-size-threshold";
+ static final String DEDICATED_SIZE_THRESHOLD_META_KEY =
+ "lance-encoding:blob-dedicated-size-threshold";
+ static final String PACK_FILE_SIZE_THRESHOLD_META_KEY =
+ "lance-encoding:blob-pack-file-size-threshold";
+
+ static final String V1 = "lance.blob.v1";
+ static final String V2 = "lance.blob.v2";
+
+ private static final String PREFIX = "lance.blob.";
+ private static final String WITH_RANGE = "with_range";
+
+ // Catalog string parameter name -> Arrow metadata key, in canonical output
order.
+ private static final Map<String, String> THRESHOLD_PARAMS =
+ ImmutableMap.of(
+ "inline_size_threshold", INLINE_SIZE_THRESHOLD_META_KEY,
+ "dedicated_size_threshold", DEDICATED_SIZE_THRESHOLD_META_KEY,
+ "pack_file_size_threshold", PACK_FILE_SIZE_THRESHOLD_META_KEY);
+
+ // Catalog string parameter name -> minimum accepted value, matching Lance's
validation.
+ private static final Map<String, Long> THRESHOLD_MINIMUMS =
+ ImmutableMap.of(
+ "inline_size_threshold", 0L,
+ "dedicated_size_threshold", 1L,
+ "pack_file_size_threshold", 1L);
+
+ private static final ArrowType UINT64 = new ArrowType.Int(64, false);
+
+ private static final List<Field> V2_MINIMAL_CHILDREN =
+ ImmutableList.of(
+ nullableChild("data", ArrowType.LargeBinary.INSTANCE),
+ nullableChild("uri", ArrowType.Utf8.INSTANCE));
+
+ private static final List<Field> V2_FULL_CHILDREN =
+ ImmutableList.<Field>builder()
+ .addAll(V2_MINIMAL_CHILDREN)
+ .add(nullableChild("position", UINT64))
+ .add(nullableChild("size", UINT64))
+ .build();
+
+ private static final String SUPPORTED_FORMATS =
+ V1
+ + ", "
+ + V2
+ + "("
+ + WITH_RANGE
+ + "=true, "
+ + String.join("=N, ", THRESHOLD_PARAMS.keySet())
+ + "=N)";
+
+ private LanceBlobTypes() {}
+
+ /**
+ * Returns whether the field carries Lance blob metadata, either legacy blob
or blob v2.
+ *
+ * @param field The Arrow field.
+ * @return true if the field is a Lance blob field.
+ */
+ static boolean isBlob(Field field) {
+ Map<String, String> metadata = field.getMetadata();
+ return metadata != null
+ && (metadata.containsKey(BLOB_META_KEY)
Review Comment:
This matches Lance: `is_blob` in `lance-arrow/src/schema.rs` checks only
that `lance-encoding:blob` is present, not its value, so Lance treats such a
field as a blob. Only the canonical `"true"` form gets `lance.blob.v1`;
anything else is kept as Arrow JSON so the blob marker survives instead of
degrading to plain `binary`.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceDataTypeConverter.java:
##########
@@ -203,7 +205,29 @@ public ArrowType fromGravitino(Type type) {
@Override
public Type toGravitino(Field arrowField) {
+ Optional<String> blobType = LanceBlobTypes.toCatalogString(arrowField);
+ if (blobType.isPresent()) {
+ return Types.ExternalType.of(blobType.get());
+ }
+
+ // Only Lance blob metadata is recognized. A blob field outside the
canonical layout keeps the
+ // whole field as Arrow JSON so the blob is not turned into a plain binary
or struct column.
+ if (LanceBlobTypes.isBlob(arrowField)) {
+ return toExternalType(arrowField);
+ }
+
FieldType fieldType = arrowField.getFieldType();
+ // List, map and union children are rebuilt with fixed names and without
metadata, so a blob
+ // below them can only be preserved by keeping the whole subtree as Arrow
JSON. Struct children
+ // keep their names and are converted recursively.
+ ArrowType.ArrowTypeID typeId = fieldType.getType().getTypeID();
+ if ((typeId == ArrowType.ArrowTypeID.List
+ || typeId == ArrowType.ArrowTypeID.Map
+ || typeId == ArrowType.ArrowTypeID.Union)
+ &&
arrowField.getChildren().stream().anyMatch(LanceDataTypeConverter::hasBlobInTree))
{
+ return toExternalType(arrowField);
+ }
Review Comment:
Added round-trip tests for `LargeList` and `FixedSizeList` with a blob v2
child.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
