Skip to content

Commit 4bec46e

Browse files
majin1102majin1102
committed
refactor: store branch and tag metadata as maps
1 parent 55559ee commit 4bec46e

11 files changed

Lines changed: 130 additions & 118 deletions

File tree

‎docs/src/format/table/branch_tag.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ Each branch metadata file is a JSON file with the following fields:
4242
| `parent_version` | number | | Version number of the parent branch at the time this branch was created. |
4343
| `create_at` | number | | Unix timestamp (seconds since epoch) when the branch was created. |
4444
| `manifest_size` | number | | Size of the initial manifest file in bytes. |
45+
| `metadata` | object | Yes | String key/value metadata map. If absent, it is treated as an empty object. |
4546

4647
### Branch Dataset Layout
4748

@@ -119,3 +120,4 @@ Each tag file is a JSON file with the following fields:
119120
| `branch` | string | Yes | Branch name being tagged. `null` or absent indicates main branch. |
120121
| `version` | number | | Version number being tagged within that branch. |
121122
| `manifest_size` | number | | Size of the manifest file in bytes. Used for efficient manifest loading. |
123+
| `metadata` | object | Yes | String key/value metadata map. If absent, it is treated as an empty object. |

‎java/lance-jni/src/blocking_dataset.rs‎

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::storage_options::JavaStorageOptionsProvider;
1111
use crate::traits::{FromJObjectWithEnv, FromJString, export_vec, import_vec, import_vec_to_rust};
1212
use crate::utils::{
1313
build_compaction_options, extract_storage_options, extract_write_params,
14-
get_scalar_index_params, get_vector_index_params, to_rust_map,
14+
get_scalar_index_params, get_vector_index_params, to_java_map, to_rust_map,
1515
};
1616
use crate::{RT, traits::IntoJava};
1717
use arrow::array::RecordBatchReader;
@@ -314,13 +314,21 @@ impl BlockingDataset {
314314
Ok(())
315315
}
316316

317-
pub fn update_tag_metadata(&mut self, tag: &str, metadata: Option<String>) -> Result<()> {
318-
RT.block_on(self.inner.tags().update_metadata(tag, metadata))?;
317+
pub fn replace_tag_metadata(
318+
&mut self,
319+
tag: &str,
320+
metadata: HashMap<String, String>,
321+
) -> Result<()> {
322+
RT.block_on(self.inner.tags().replace_metadata(tag, metadata))?;
319323
Ok(())
320324
}
321325

322-
pub fn update_branch_metadata(&mut self, branch: &str, metadata: Option<String>) -> Result<()> {
323-
RT.block_on(self.inner.branches().update_metadata(branch, metadata))?;
326+
pub fn replace_branch_metadata(
327+
&mut self,
328+
branch: &str,
329+
metadata: HashMap<String, String>,
330+
) -> Result<()> {
331+
RT.block_on(self.inner.branches().replace_metadata(branch, metadata))?;
324332
Ok(())
325333
}
326334

@@ -2388,11 +2396,10 @@ fn inner_list_tags<'local>(
23882396
} else {
23892397
JObject::null()
23902398
};
2391-
let java_metadata =
2392-
crate::traits::JLance(tag_contents.metadata.as_deref()).into_java(env)?;
2399+
let java_metadata = to_java_map(env, &tag_contents.metadata)?;
23932400
let java_tag = env.new_object(
23942401
"org/lance/Tag",
2395-
"(Ljava/lang/String;Ljava/lang/String;JILjava/lang/String;)V",
2402+
"(Ljava/lang/String;Ljava/lang/String;JILjava/util/Map;)V",
23962403
&[
23972404
JValue::Object(&env.new_string(tag_name)?.into()),
23982405
JValue::Object(&branch_name),
@@ -2481,29 +2488,29 @@ fn inner_update_tag(
24812488
}
24822489

24832490
#[unsafe(no_mangle)]
2484-
pub extern "system" fn Java_org_lance_Dataset_nativeUpdateTagMetadata(
2491+
pub extern "system" fn Java_org_lance_Dataset_nativeReplaceTagMetadata(
24852492
mut env: JNIEnv,
24862493
java_dataset: JObject,
24872494
jtag_name: JString,
24882495
jmetadata: JObject,
24892496
) {
24902497
ok_or_throw_without_return!(
24912498
env,
2492-
inner_update_tag_metadata(&mut env, java_dataset, jtag_name, jmetadata)
2499+
inner_replace_tag_metadata(&mut env, java_dataset, jtag_name, jmetadata)
24932500
)
24942501
}
24952502

2496-
fn inner_update_tag_metadata(
2503+
fn inner_replace_tag_metadata(
24972504
env: &mut JNIEnv,
24982505
java_dataset: JObject,
24992506
jtag_name: JString,
25002507
jmetadata: JObject,
25012508
) -> Result<()> {
25022509
let tag = jtag_name.extract(env)?;
2503-
let metadata = extract_optional_metadata(env, &jmetadata)?;
2510+
let metadata = extract_metadata_map(env, &jmetadata)?;
25042511
let mut dataset_guard =
25052512
{ unsafe { env.get_rust_field::<_, _, BlockingDataset>(java_dataset, NATIVE_DATASET) }? };
2506-
dataset_guard.update_tag_metadata(tag.as_str(), metadata)
2513+
dataset_guard.replace_tag_metadata(tag.as_str(), metadata)
25072514
}
25082515

25092516
#[unsafe(no_mangle)]
@@ -2560,10 +2567,10 @@ fn inner_list_branches<'local>(
25602567
} else {
25612568
JObject::null()
25622569
};
2563-
let java_metadata = crate::traits::JLance(contents.metadata.as_deref()).into_java(env)?;
2570+
let java_metadata = to_java_map(env, &contents.metadata)?;
25642571
let jbranch = env.new_object(
25652572
"org/lance/Branch",
2566-
"(Ljava/lang/String;Ljava/lang/String;JJILjava/lang/String;)V",
2573+
"(Ljava/lang/String;Ljava/lang/String;JJILjava/util/Map;)V",
25672574
&[
25682575
JValue::Object(&jname),
25692576
JValue::Object(&jparent),
@@ -2622,36 +2629,34 @@ fn inner_create_branch<'local>(
26222629
}
26232630

26242631
#[unsafe(no_mangle)]
2625-
pub extern "system" fn Java_org_lance_Dataset_nativeUpdateBranchMetadata(
2632+
pub extern "system" fn Java_org_lance_Dataset_nativeReplaceBranchMetadata(
26262633
mut env: JNIEnv,
26272634
java_dataset: JObject,
26282635
jbranch: JString,
26292636
jmetadata: JObject,
26302637
) {
26312638
ok_or_throw_without_return!(
26322639
env,
2633-
inner_update_branch_metadata(&mut env, java_dataset, jbranch, jmetadata)
2640+
inner_replace_branch_metadata(&mut env, java_dataset, jbranch, jmetadata)
26342641
)
26352642
}
26362643

2637-
fn inner_update_branch_metadata(
2644+
fn inner_replace_branch_metadata(
26382645
env: &mut JNIEnv,
26392646
java_dataset: JObject,
26402647
jbranch: JString,
26412648
jmetadata: JObject,
26422649
) -> Result<()> {
26432650
let branch: String = jbranch.extract(env)?;
2644-
let metadata = extract_optional_metadata(env, &jmetadata)?;
2651+
let metadata = extract_metadata_map(env, &jmetadata)?;
26452652
let mut dataset_guard =
26462653
unsafe { env.get_rust_field::<_, _, BlockingDataset>(java_dataset, NATIVE_DATASET) }?;
2647-
dataset_guard.update_branch_metadata(&branch, metadata)
2654+
dataset_guard.replace_branch_metadata(&branch, metadata)
26482655
}
26492656

2650-
fn extract_optional_metadata(env: &mut JNIEnv, jmetadata: &JObject) -> Result<Option<String>> {
2651-
env.get_optional(jmetadata, |env, metadata_obj| {
2652-
let metadata = JString::from(metadata_obj);
2653-
Ok(env.get_string(&metadata)?.into())
2654-
})
2657+
fn extract_metadata_map(env: &mut JNIEnv, jmetadata: &JObject) -> Result<HashMap<String, String>> {
2658+
let jmap = JMap::from_env(env, jmetadata)?;
2659+
to_rust_map(env, &jmap)
26552660
}
26562661

26572662
fn transform_jref_to_ref(jref: JObject, env: &mut JNIEnv) -> Result<Ref> {

‎java/src/main/java/org/lance/Branch.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
import com.google.common.base.MoreObjects;
1717

18+
import java.util.Collections;
19+
import java.util.HashMap;
20+
import java.util.Map;
1821
import java.util.Objects;
1922
import java.util.Optional;
2023

@@ -30,11 +33,11 @@ public class Branch {
3033
private final long parentVersion;
3134
private final long createAt;
3235
private final int manifestSize;
33-
private final Optional<String> metadata;
36+
private final Map<String, String> metadata;
3437

3538
public Branch(
3639
String name, String parentBranch, long parentVersion, long createAt, int manifestSize) {
37-
this(name, parentBranch, parentVersion, createAt, manifestSize, null);
40+
this(name, parentBranch, parentVersion, createAt, manifestSize, Collections.emptyMap());
3841
}
3942

4043
public Branch(
@@ -43,13 +46,13 @@ public Branch(
4346
long parentVersion,
4447
long createAt,
4548
int manifestSize,
46-
String metadata) {
49+
Map<String, String> metadata) {
4750
this.name = name;
4851
this.parentBranch = Optional.ofNullable(parentBranch);
4952
this.parentVersion = parentVersion;
5053
this.createAt = createAt;
5154
this.manifestSize = manifestSize;
52-
this.metadata = Optional.ofNullable(metadata);
55+
this.metadata = Collections.unmodifiableMap(new HashMap<>(metadata));
5356
}
5457

5558
public String getName() {
@@ -72,7 +75,7 @@ public int getManifestSize() {
7275
return manifestSize;
7376
}
7477

75-
public Optional<String> getMetadata() {
78+
public Map<String, String> getMetadata() {
7679
return metadata;
7780
}
7881

‎java/src/main/java/org/lance/Dataset.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,11 +1686,12 @@ public void update(String tag, Ref ref) {
16861686
}
16871687
}
16881688

1689-
public void updateMetadata(String tag, Optional<String> metadata) {
1689+
public void replaceMetadata(String tag, Map<String, String> metadata) {
16901690
Preconditions.checkArgument(tag != null, "tag cannot be null");
1691+
Preconditions.checkArgument(metadata != null, "metadata cannot be null");
16911692
try (LockManager.WriteLock writeLock = lockManager.acquireWriteLock()) {
16921693
Preconditions.checkArgument(nativeDatasetHandle != 0, "Dataset is closed");
1693-
nativeUpdateTagMetadata(tag, metadata);
1694+
nativeReplaceTagMetadata(tag, metadata);
16941695
}
16951696
}
16961697

@@ -1747,11 +1748,12 @@ public List<Branch> list() {
17471748
}
17481749
}
17491750

1750-
public void updateMetadata(String branchName, Optional<String> metadata) {
1751+
public void replaceMetadata(String branchName, Map<String, String> metadata) {
17511752
Preconditions.checkArgument(branchName != null, "branchName cannot be null");
1753+
Preconditions.checkArgument(metadata != null, "metadata cannot be null");
17521754
try (LockManager.WriteLock writeLock = lockManager.acquireWriteLock()) {
17531755
Preconditions.checkArgument(nativeDatasetHandle != 0, "Dataset is closed");
1754-
nativeUpdateBranchMetadata(branchName, metadata);
1756+
nativeReplaceBranchMetadata(branchName, metadata);
17551757
}
17561758
}
17571759
}
@@ -1839,7 +1841,7 @@ private native MergeInsertResult nativeMergeInsert(
18391841

18401842
private native void nativeUpdateTag(String tag, Ref ref);
18411843

1842-
private native void nativeUpdateTagMetadata(String tag, Optional<String> metadata);
1844+
private native void nativeReplaceTagMetadata(String tag, Map<String, String> metadata);
18431845

18441846
private native List<Tag> nativeListTags();
18451847

@@ -1855,7 +1857,7 @@ private native Dataset nativeCreateBranch(
18551857

18561858
private native List<Branch> nativeListBranches();
18571859

1858-
private native void nativeUpdateBranchMetadata(String branch, Optional<String> metadata);
1860+
private native void nativeReplaceBranchMetadata(String branch, Map<String, String> metadata);
18591861

18601862
public Dataset shallowClone(String targetPath, Ref ref) {
18611863
return shallowClone(targetPath, ref, null);

‎java/src/main/java/org/lance/Tag.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
import com.google.common.base.MoreObjects;
1717

18+
import java.util.Collections;
19+
import java.util.HashMap;
20+
import java.util.Map;
1821
import java.util.Objects;
1922
import java.util.Optional;
2023

@@ -23,18 +26,19 @@ public class Tag {
2326
private final Optional<String> branch;
2427
private final long version;
2528
private final int manifestSize;
26-
private final Optional<String> metadata;
29+
private final Map<String, String> metadata;
2730

2831
public Tag(String name, String branch, long version, int manifestSize) {
29-
this(name, branch, version, manifestSize, null);
32+
this(name, branch, version, manifestSize, Collections.emptyMap());
3033
}
3134

32-
public Tag(String name, String branch, long version, int manifestSize, String metadata) {
35+
public Tag(
36+
String name, String branch, long version, int manifestSize, Map<String, String> metadata) {
3337
this.name = name;
3438
this.branch = Optional.ofNullable(branch);
3539
this.version = version;
3640
this.manifestSize = manifestSize;
37-
this.metadata = Optional.ofNullable(metadata);
41+
this.metadata = Collections.unmodifiableMap(new HashMap<>(metadata));
3842
}
3943

4044
public String getName() {
@@ -53,7 +57,7 @@ public int getManifestSize() {
5357
return manifestSize;
5458
}
5559

56-
public Optional<String> getMetadata() {
60+
public Map<String, String> getMetadata() {
5761
return metadata;
5862
}
5963

‎java/src/test/java/org/lance/DatasetTest.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,11 @@ void testTags(@TempDir Path tempDir) {
317317
try (Dataset dataset = testDataset.createEmptyDataset()) {
318318
assertEquals(1, dataset.version());
319319
dataset.tags().create("tag1", Ref.ofMain());
320-
dataset.tags().updateMetadata("tag1", Optional.of("primary tag"));
320+
dataset.tags().replaceMetadata("tag1", Map.of("description", "primary tag"));
321321
assertEquals(1, dataset.tags().list().size());
322322
assertEquals(1, dataset.tags().list().get(0).getVersion());
323-
assertEquals(Optional.of("primary tag"), dataset.tags().list().get(0).getMetadata());
323+
assertEquals(
324+
Map.of("description", "primary tag"), dataset.tags().list().get(0).getMetadata());
324325
assertEquals(1, dataset.tags().getVersion("tag1"));
325326
}
326327

@@ -334,14 +335,14 @@ void testTags(@TempDir Path tempDir) {
334335
assertEquals(2, dataset2.tags().list().size());
335336
assertEquals(1, dataset2.tags().getVersion("tag1"));
336337
assertEquals(2, dataset2.tags().getVersion("tag2"));
337-
dataset2.tags().updateMetadata("tag2", Optional.of("rollback tag"));
338+
dataset2.tags().replaceMetadata("tag2", Map.of("description", "rollback tag"));
338339
assertEquals(2, dataset2.tags().list().size());
339340
assertEquals(1, dataset2.tags().list().get(0).getVersion());
340341
assertEquals(2, dataset2.tags().list().get(1).getVersion());
341342
assertEquals(1, dataset2.tags().getVersion("tag1"));
342343
assertEquals(2, dataset2.tags().getVersion("tag2"));
343344
assertEquals(
344-
Optional.of("rollback tag"),
345+
Map.of("description", "rollback tag"),
345346
dataset2.tags().list().stream()
346347
.filter(tag -> tag.getName().equals("tag2"))
347348
.findFirst()
@@ -350,15 +351,15 @@ void testTags(@TempDir Path tempDir) {
350351
dataset2.tags().update("tag2", Ref.ofMain(1));
351352
assertEquals(1, dataset2.tags().getVersion("tag2"));
352353
assertEquals(
353-
Optional.of("rollback tag"),
354+
Map.of("description", "rollback tag"),
354355
dataset2.tags().list().stream()
355356
.filter(tag -> tag.getName().equals("tag2"))
356357
.findFirst()
357358
.orElseThrow()
358359
.getMetadata());
359-
dataset2.tags().updateMetadata("tag2", Optional.empty());
360+
dataset2.tags().replaceMetadata("tag2", Collections.emptyMap());
360361
assertEquals(
361-
Optional.empty(),
362+
Collections.emptyMap(),
362363
dataset2.tags().list().stream()
363364
.filter(tag -> tag.getName().equals("tag2"))
364365
.findFirst()
@@ -383,7 +384,7 @@ void testTags(@TempDir Path tempDir) {
383384

384385
try (Dataset branch = dataset2.createBranch("branch", Ref.ofMain(2))) {
385386
branch.tags().create("tag_on_branch", Ref.ofBranch("branch"));
386-
branch.tags().updateMetadata("tag_on_branch", Optional.of("branch tag"));
387+
branch.tags().replaceMetadata("tag_on_branch", Map.of("description", "branch tag"));
387388
assertEquals(2, dataset2.tags().getVersion("tag_on_branch"));
388389
List<Tag> tags = dataset2.tags().list();
389390
Optional<Tag> tagOptional =
@@ -394,7 +395,7 @@ void testTags(@TempDir Path tempDir) {
394395
assertTrue(tagOptional.isPresent());
395396
assertEquals(2, tagOptional.get().getVersion());
396397
assertEquals(Optional.of("branch"), tagOptional.get().getBranch());
397-
assertEquals(Optional.of("branch tag"), tagOptional.get().getMetadata());
398+
assertEquals(Map.of("description", "branch tag"), tagOptional.get().getMetadata());
398399

399400
dataset2.tags().update("tag1", Ref.ofBranch("branch"));
400401
tags = dataset2.tags().list();
@@ -1739,17 +1740,19 @@ void testBranches(@TempDir Path tempDir) {
17391740
assertFalse(branch1Meta.getParentBranch().isPresent());
17401741
assertTrue(branch1Meta.getCreateAt() > 0);
17411742
assertTrue(branch1Meta.getManifestSize() > 0);
1742-
assertEquals(Optional.empty(), branch1Meta.getMetadata());
1743-
mainV2.branches().updateMetadata("branch1", Optional.of("long-lived branch"));
1743+
assertEquals(Collections.emptyMap(), branch1Meta.getMetadata());
1744+
mainV2
1745+
.branches()
1746+
.replaceMetadata("branch1", Map.of("description", "long-lived branch"));
17441747
branches = branch2V4.branches().list();
17451748
b1 = branches.stream().filter(b -> b.getName().equals("branch1")).findFirst();
17461749
assertTrue(b1.isPresent());
1747-
assertEquals(Optional.of("long-lived branch"), b1.get().getMetadata());
1748-
mainV2.branches().updateMetadata("branch1", Optional.empty());
1750+
assertEquals(Map.of("description", "long-lived branch"), b1.get().getMetadata());
1751+
mainV2.branches().replaceMetadata("branch1", Collections.emptyMap());
17491752
branches = branch2V4.branches().list();
17501753
b1 = branches.stream().filter(b -> b.getName().equals("branch1")).findFirst();
17511754
assertTrue(b1.isPresent());
1752-
assertEquals(Optional.empty(), b1.get().getMetadata());
1755+
assertEquals(Collections.emptyMap(), b1.get().getMetadata());
17531756

17541757
assertEquals("branch2", branch2Meta.getName());
17551758
assertTrue(branch2Meta.getParentBranch().isPresent());

0 commit comments

Comments
 (0)