FANNG1 commented on code in PR #13390:
URL: https://github.com/apache/gravitino/pull/13390#discussion_r4100861762
##########
docs/lakehouse-generic-lance-table.md:
##########
@@ -97,6 +99,38 @@ 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. Blob fields nested in a
+`Struct`, `List`, `Map` or `Union` keep the container as a native Gravitino
type, for example
+`List(External("lance.blob.v2"))`. As with other list columns, the list child
is written back as
+`element`, whatever name it had in the Lance dataset (Lance and pyarrow use
`item`).
+
+### Blob Types
+
+Lance blob columns use a readable external type instead of Arrow JSON:
+
+| External Type Definition | Arrow Field
|
+|----------------------------------------------|---------------------------------------------------------------------------------------------------------|
+| `External("lance.blob.v1")` | `LargeBinary` with metadata
`lance-encoding:blob=true` |
+| `External("lance.blob.v2")` | `Struct<data: LargeBinary,
uri: Utf8>` with metadata `ARROW:extension:name=lance.blob.v2` |
+| `External("lance.blob.v2(with_range=true)")` | `Struct<data: LargeBinary,
uri: Utf8, position: UInt64, size: UInt64>` with the same extension metadata |
+
+`lance.blob.v2` accepts these optional parameters, written as
`lance.blob.v2(key=value, ...)`:
+
+| Parameter | Arrow Field Metadata
| Value |
+|----------------------------|------------------------------------------------|-------------------------------------------------------------|
+| `with_range` | -
| `true` or `false` (default), adds `position` and `size` |
+| `inline_size_threshold` | `lance-encoding:blob-inline-size-threshold`
| Integer >= 0 |
+| `dedicated_size_threshold` | `lance-encoding:blob-dedicated-size-threshold`
| Integer > 0 |
+| `pack_file_size_threshold` | `lance-encoding:blob-pack-file-size-threshold`
| Integer > 0 |
+
+For example, `External("lance.blob.v2(inline_size_threshold=4096,
dedicated_size_threshold=1048576)")`.
+
+Legacy blob columns are rejected by Lance for file version 2.2 and later; use
blob v2 for new tables.
Review Comment:
Fixed. `createDataset` now sets `dataStorageVersion` to 2.2 for `lance.blob`
and 2.1 for `lance.blob.legacy`, rejects mixing them, and AddColumn checks
`getLanceFileFormatVersion()` up front with a clear message. Added real create
→ load tests for both, plus the AddColumn cases.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceBlobTypes.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * 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 =
Review Comment:
Agreed, and thresholds are storage settings, not type semantics: on append
Lance takes them from the dataset's stored schema, so dropping them from the
Gravitino type does not change how existing datasets write. Removed all
parameters; the type is just `lance.blob`.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceBlobTypes.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * 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 =
Review Comment:
Gone with the parameters.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceBlobTypes.java:
##########
@@ -0,0 +1,336 @@
+/*
+ * 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:
Kept on purpose: Lance 6.0.0's `is_blob` (`lance-arrow/src/schema.rs`) only
checks that the key is present, so Lance treats `lance-encoding:blob=false` as
a blob. Mapping it to `BinaryType` would drop the marker; Arrow JSON keeps it
exact. Only `"true"` on `LargeBinary` becomes `lance.blob.legacy`.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceDataTypeConverter.java:
##########
@@ -113,24 +114,25 @@ public Field toArrowField(String name, Type type, boolean
nullable) {
case EXTERNAL:
Types.ExternalType externalType = (Types.ExternalType) type;
+ if (LanceBlobTypes.isBlobCatalogString(externalType.catalogString())) {
+ return LanceBlobTypes.toArrowField(name, nullable,
externalType.catalogString());
Review Comment:
No longer needed: with no parameters there is only one spelling per type
(surrounding whitespace is trimmed).
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceDataTypeConverter.java:
##########
@@ -113,24 +114,25 @@ public Field toArrowField(String name, Type type, boolean
nullable) {
case EXTERNAL:
Types.ExternalType externalType = (Types.ExternalType) type;
+ if (LanceBlobTypes.isBlobCatalogString(externalType.catalogString())) {
+ return LanceBlobTypes.toArrowField(name, nullable,
externalType.catalogString());
+ }
Field field;
try {
field = mapper.readValue(externalType.catalogString(), Field.class);
} catch (Exception e) {
throw new RuntimeException(
"Failed to parse external type catalog string: " +
externalType.catalogString(), e);
}
- Preconditions.checkArgument(
- name.equals(field.getName()),
- "expected field name %s but got %s",
- name,
- field.getName());
Preconditions.checkArgument(
nullable == field.isNullable(),
"expected field nullable %s but got %s",
nullable,
field.isNullable());
- return field;
+ // The column name is authoritative: a renamed column keeps its stored
JSON type.
Review Comment:
Called out separately in the PR description.
##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/LanceDataTypeConverter.java:
##########
@@ -203,7 +205,19 @@ public ArrowType fromGravitino(Type type) {
@Override
public Type toGravitino(Field arrowField) {
+ Optional<String> blobType = LanceBlobTypes.toCatalogString(arrowField);
Review Comment:
Added a note to the docs covering blob v2 columns loaded as `Struct` before
this change and legacy blob columns switching from Arrow JSON to
`lance.blob.legacy`.
--
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]
