Skip to content

Commit a6d5ffc

Browse files
authored
Added fix for malformed warehouse file path (#133)
1 parent 8e67373 commit a6d5ffc

5 files changed

Lines changed: 95 additions & 29 deletions

File tree

examples/localfileio/.ice-rest-catalog.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
uri: jdbc:sqlite:file:data/ice-rest-catalog/catalog.sqlite?journal_mode=WAL&synchronous=OFF&journal_size_limit=500
2-
warehouse: file://warehouse
3-
localFileIOBaseDir: data/ice-rest-catalog
2+
warehouse: file:///tmp/ice-example/warehouse
43
anonymousAccess:
54
enabled: true
65
accessConfig:

examples/localfileio/.ice.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
warehouse: file:///tmp/ice-example/warehouse
2+
13
uri: http://localhost:5000
2-
localFileIOBaseDir: data/ice-rest-catalog
34
httpCacheDir: data/ice/http/cache

examples/localfileio/README.md

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
# examples/localfileio
22

3-
This example is primarily intended for learning and experimentation.
4-
All data is stored in data/ directory as regular files.
3+
This is an example setup where Iceberg table data is stored on your local disk (under /tmp/ice-example/warehouse) instead of
4+
in cloud object storage (S3, GCS, etc.).
5+
This example is primarily intended for learning and experimentation, and development without cloud credentials.
6+
Table data is stored under `/tmp/ice-example/warehouse` as regular files (see `warehouse` in `.ice-rest-catalog.yaml`). The catalog metadata stays under `data/ice-rest-catalog/`.
57

68
```shell
79
# optional: open shell containing `sqlite3` (sqlite command line client)
810
devbox shell
911

10-
# start Iceberg REST Catalog server backed by sqlite with warehouse set to file://warehouse
12+
# start Iceberg REST Catalog server backed by sqlite with warehouse set to file:///tmp/ice-example/warehouse
1113
ice-rest-catalog
1214

1315
# insert data into catalog
@@ -17,7 +19,7 @@ ice insert flowers.iris -p file://iris.parquet
1719
ice describe
1820

1921
# list all warehouse files
20-
find data/ice-rest-catalog/warehouse
22+
find /tmp/ice-example/warehouse
2123

2224
# inspect sqlite data
2325
sqlite3 data/ice-rest-catalog/catalog.sqlite
@@ -28,12 +30,18 @@ sqlite> select * from iceberg_tables;
2830
sqlite> select * from iceberg_namespace_properties;
2931
sqlite> .quit
3032

31-
# open ClickHouse* shell, then try SQL below
32-
docker run -it --rm --network host -v $(pwd)/data/ice-rest-catalog/warehouse:/warehouse \
33-
altinity/clickhouse-server:25.3.3.20186.altinityantalya clickhouse local
33+
# open ClickHouse* shell, then try SQL below
34+
# IMPORTANT: make sure mount path is set to /tmp
35+
# For Linux:
36+
docker run -it --rm --add-host=host.docker.internal:host-gateway --network host -v /tmp/ice-example/warehouse:/tmp/ice-example/warehouse \
37+
altinity/clickhouse-server:25.8.16.20002.altinityantalya clickhouse local
38+
# For Mac:
39+
docker run -it --rm --network host -v /tmp/ice-example/warehouse:/tmp/ice-example/warehouse \
40+
altinity/clickhouse-server:25.8.16.20002.altinityantalya clickhouse local
41+
3442
```
3543
36-
> \* currently this only works with altinity/clickhouse-server:25.3+ builds.
44+
> currently this only works with altinity/clickhouse-server:25.3+ builds.
3745
3846
```sql
3947
-- enable Iceberg support (required as of 25.4.1.1795)
@@ -43,18 +51,50 @@ SET allow_experimental_database_iceberg = 1;
4351
DROP DATABASE IF EXISTS ice;
4452

4553
CREATE DATABASE ice
46-
ENGINE = DataLakeCatalog('http://localhost:5000')
54+
ENGINE = DataLakeCatalog('http://host.docker.internal:5000')
4755
SETTINGS catalog_type = 'rest',
4856
vended_credentials = false,
4957
warehouse = 'warehouse';
5058

51-
SHOW TABLES FROM ice;
52-
5359
-- inspect
5460
SHOW DATABASES;
5561
SHOW TABLES FROM ice;
62+
63+
64+
Query id: 8c671684-ec90-4e18-aacd-aee5fae2aeed
65+
66+
┌─name──────────┐
67+
1. │ flowers.iris │
68+
69+
70+
5671
SHOW CREATE TABLE ice.`flowers.iris`;
5772

5873
select count(*) from ice.`flowers.iris`;
5974
select * from ice.`flowers.iris` limit 10 FORMAT CSVWithNamesAndTypes;
75+
76+
Query id: f3a1773d-ebeb-4d26-80fb-7d9e29b5c07e
77+
78+
┌─sepal.length─┬─sepal.width─┬─petal.length─┬─petal.width─┬─variety────┐
79+
1. │ 5.1 │ 3.5 │ 1.4 │ 0.2 │ Setosa │
80+
2. │ 4.9 │ 3 │ 1.4 │ 0.2 │ Setosa │
81+
3. │ 4.7 │ 3.2 │ 1.3 │ 0.2 │ Setosa │
82+
4. │ 4.6 │ 3.1 │ 1.5 │ 0.2 │ Setosa │
83+
5. │ 5 │ 3.6 │ 1.4 │ 0.2 │ Setosa │
84+
6. │ 5.4 │ 3.9 │ 1.7 │ 0.4 │ Setosa │
85+
7. │ 4.6 │ 3.4 │ 1.4 │ 0.3 │ Setosa │
86+
8. │ 5 │ 3.4 │ 1.5 │ 0.2 │ Setosa │
87+
9. │ 4.4 │ 2.9 │ 1.4 │ 0.2 │ Setosa │
88+
10. │ 4.9 │ 3.1 │ 1.5 │ 0.1 │ Setosa │
89+
11. │ 5.4 │ 3.7 │ 1.5 │ 0.2 │ Setosa │
90+
12. │ 4.8 │ 3.4 │ 1.6 │ 0.2 │ Setosa │
91+
13. │ 4.8 │ 3 │ 1.4 │ 0.1 │ Setosa │
92+
14. │ 4.3 │ 3 │ 1.1 │ 0.1 │ Setosa │
93+
15. │ 5.8 │ 4 │ 1.2 │ 0.2 │ Setosa │
94+
16. │ 5.7 │ 4.4 │ 1.5 │ 0.4 │ Setosa │
95+
17. │ 5.4 │ 3.9 │ 1.3 │ 0.4 │ Setosa │
96+
18. │ 5.1 │ 3.5 │ 1.4 │ 0.3 │ Setosa │
97+
19. │ 5.7 │ 3.8 │ 1.7 │
6098
```
99+
100+
The REST catalog `warehouse` must be an absolute `file:///` URI (e.g. `file:///tmp/ice-example/warehouse`). A relative form like `file://warehouse` is rejected by the server.

ice/src/main/java/com/altinity/ice/internal/iceberg/io/LocalFileIO.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ public void initialize(Map<String, String> properties) {
5757
warehouse.startsWith("file://"),
5858
"\"%s\" must start with file://",
5959
LOCALFILEIO_PROP_WAREHOUSE);
60+
Preconditions.checkArgument(
61+
warehouse.startsWith("file:///") || warehouse.equals("file://"),
62+
"\"%s\" must use an absolute path (file:///abs/path), got: %s",
63+
LOCALFILEIO_PROP_WAREHOUSE,
64+
warehouse);
6065
this.workDir = resolveWorkdir(warehouse, properties.get(LOCALFILEIO_PROP_BASEDIR));
6166
Path warehousePath = resolveWarehousePath(warehouse, workDir);
6267
if (!Files.isDirectory(warehousePath)) {

ice/src/test/java/com/altinity/ice/internal/iceberg/io/LocalFileIOIT.java

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1414

1515
import com.altinity.ice.internal.strings.Strings;
16-
import java.io.File;
1716
import java.io.IOException;
1817
import java.nio.file.Files;
1918
import java.nio.file.Path;
19+
import java.nio.file.Paths;
2020
import java.util.Comparator;
21+
import java.util.HashMap;
2122
import java.util.List;
2223
import java.util.Map;
2324
import java.util.function.Function;
@@ -58,14 +59,23 @@ public void tearDown() throws IOException {
5859

5960
@Test
6061
public void testBasicFlow() throws IOException {
61-
for (var warehouse : new String[] {"file://.", "file://", "file://x/y/z"}) {
62-
tempDir.toFile().mkdirs();
63-
new File(tempDir.toString(), Strings.removePrefix(warehouse, "file://")).mkdirs();
64-
Path fooFile = tempDir.resolve(Strings.removePrefix(warehouse, "file://")).resolve("foo");
65-
Files.writeString(fooFile, "foo_content");
66-
Files.writeString(
67-
tempDir.resolve(Strings.removePrefix(warehouse, "file://")).resolve("bar"),
68-
"bar_content");
62+
Path absTemp = tempDir.toAbsolutePath().normalize();
63+
String[] warehouses =
64+
new String[] {
65+
absTemp.toUri().toString(), "file://", absTemp.resolve("x/y/z").toUri().toString()
66+
};
67+
68+
for (String warehouse : warehouses) {
69+
Path warehousePhysical =
70+
"file://".equals(warehouse)
71+
? absTemp
72+
: Paths.get(Strings.removePrefix(warehouse, "file://"));
73+
Files.createDirectories(warehousePhysical);
74+
Files.writeString(warehousePhysical.resolve("foo"), "foo_content");
75+
Files.writeString(warehousePhysical.resolve("bar"), "bar_content");
76+
77+
Path fooFile = warehousePhysical.resolve("foo");
78+
6979
try (LocalFileIO io = new LocalFileIO()) {
7080
assertThatThrownBy(
7181
() ->
@@ -81,12 +91,12 @@ public void testBasicFlow() throws IOException {
8191
Function<String, String> warehouseLocation =
8292
(String s) -> (warehouse.endsWith("/") ? warehouse : warehouse + "/") + s;
8393

84-
io.initialize(
85-
Map.of(
86-
LocalFileIO.LOCALFILEIO_PROP_BASEDIR,
87-
tempDir.toString(),
88-
LocalFileIO.LOCALFILEIO_PROP_WAREHOUSE,
89-
warehouse));
94+
Map<String, String> props = new HashMap<>();
95+
props.put(LocalFileIO.LOCALFILEIO_PROP_WAREHOUSE, warehouse);
96+
if ("file://".equals(warehouse)) {
97+
props.put(LocalFileIO.LOCALFILEIO_PROP_BASEDIR, absTemp.toString());
98+
}
99+
io.initialize(props);
90100

91101
InputFile inputFile = io.newInputFile("foo");
92102
OutputFile outputFile = io.newOutputFile("foo.out");
@@ -146,4 +156,15 @@ public void testBasicFlow() throws IOException {
146156
}
147157
}
148158
}
159+
160+
@Test
161+
public void testRelativeFileUriRejected() {
162+
try (LocalFileIO io = new LocalFileIO()) {
163+
assertThatThrownBy(
164+
() ->
165+
io.initialize(Map.of(LocalFileIO.LOCALFILEIO_PROP_WAREHOUSE, "file://warehouse")))
166+
.isInstanceOf(IllegalArgumentException.class)
167+
.hasMessageContaining("absolute path");
168+
}
169+
}
149170
}

0 commit comments

Comments
 (0)