Skip to content

Commit f0c8ad2

Browse files
nastranastra
committed
Core: Pass namespace separator via query param
1 parent 1c52847 commit f0c8ad2

File tree

10 files changed

+354
-45
lines changed

10 files changed

+354
-45
lines changed

‎core/src/main/java/org/apache/iceberg/rest/HTTPClient.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,17 @@ public <T extends RESTResponse> T post(
380380
Method.POST, path, null, body, responseType, headers, errorHandler, responseHeaders);
381381
}
382382

383+
@Override
384+
public <T extends RESTResponse> T post(
385+
String path,
386+
RESTRequest body,
387+
Map<String, String> queryParams,
388+
Class<T> responseType,
389+
Map<String, String> headers,
390+
Consumer<ErrorResponse> errorHandler) {
391+
return execute(Method.POST, path, queryParams, body, responseType, headers, errorHandler);
392+
}
393+
383394
@Override
384395
public <T extends RESTResponse> T delete(
385396
String path,

‎core/src/main/java/org/apache/iceberg/rest/RESTClient.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,30 @@ <T extends RESTResponse> T post(
143143
Map<String, String> headers,
144144
Consumer<ErrorResponse> errorHandler);
145145

146+
default <T extends RESTResponse> T post(
147+
String path,
148+
RESTRequest body,
149+
Map<String, String> queryParams,
150+
Class<T> responseType,
151+
Map<String, String> headers,
152+
Consumer<ErrorResponse> errorHandler) {
153+
if (null != queryParams && !queryParams.isEmpty()) {
154+
throw new UnsupportedOperationException("Query params are not supported");
155+
}
156+
157+
return post(path, body, responseType, headers, errorHandler);
158+
}
159+
160+
default <T extends RESTResponse> T post(
161+
String path,
162+
RESTRequest body,
163+
Map<String, String> queryParams,
164+
Class<T> responseType,
165+
Supplier<Map<String, String>> headers,
166+
Consumer<ErrorResponse> errorHandler) {
167+
return post(path, body, queryParams, responseType, headers.get(), errorHandler);
168+
}
169+
146170
default <T extends RESTResponse> T postForm(
147171
String path,
148172
Map<String, String> formData,

‎core/src/main/java/org/apache/iceberg/rest/RESTMetricsReporter.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,17 @@ class RESTMetricsReporter implements MetricsReporter {
3636
private final RESTClient client;
3737
private final String metricsEndpoint;
3838
private final Supplier<Map<String, String>> headers;
39+
private final Map<String, String> queryParams;
3940

4041
RESTMetricsReporter(
41-
RESTClient client, String metricsEndpoint, Supplier<Map<String, String>> headers) {
42+
RESTClient client,
43+
String metricsEndpoint,
44+
Supplier<Map<String, String>> headers,
45+
Map<String, String> queryParams) {
4246
this.client = client;
4347
this.metricsEndpoint = metricsEndpoint;
4448
this.headers = headers;
49+
this.queryParams = queryParams;
4550
}
4651

4752
@Override
@@ -55,6 +60,7 @@ public void report(MetricsReport report) {
5560
client.post(
5661
metricsEndpoint,
5762
ReportMetricsRequest.of(report),
63+
queryParams,
5864
null,
5965
headers,
6066
ErrorHandlers.defaultErrorHandler());

‎core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ public List<TableIdentifier> listTables(SessionContext context, Namespace ns) {
333333
ListTablesResponse response =
334334
client.get(
335335
paths.tables(ns),
336-
queryParams,
336+
queryParams(queryParams),
337337
ListTablesResponse.class,
338338
headers(context),
339339
ErrorHandlers.namespaceErrorHandler());
@@ -350,7 +350,11 @@ public boolean dropTable(SessionContext context, TableIdentifier identifier) {
350350

351351
try {
352352
client.delete(
353-
paths.table(identifier), null, headers(context), ErrorHandlers.tableErrorHandler());
353+
paths.table(identifier),
354+
queryParams(),
355+
null,
356+
headers(context),
357+
ErrorHandlers.tableErrorHandler());
354358
return true;
355359
} catch (NoSuchTableException e) {
356360
return false;
@@ -364,7 +368,7 @@ public boolean purgeTable(SessionContext context, TableIdentifier identifier) {
364368
try {
365369
client.delete(
366370
paths.table(identifier),
367-
ImmutableMap.of("purgeRequested", "true"),
371+
queryParams(ImmutableMap.of("purgeRequested", "true")),
368372
null,
369373
headers(context),
370374
ErrorHandlers.tableErrorHandler());
@@ -390,7 +394,7 @@ private LoadTableResponse loadInternal(
390394
SessionContext context, TableIdentifier identifier, SnapshotMode mode) {
391395
return client.get(
392396
paths.table(identifier),
393-
mode.params(),
397+
queryParams(mode.params()),
394398
LoadTableResponse.class,
395399
headers(context),
396400
ErrorHandlers.tableErrorHandler());
@@ -450,6 +454,7 @@ public Table loadTable(SessionContext context, TableIdentifier identifier) {
450454
new RESTTableOperations(
451455
client,
452456
paths.table(finalIdentifier),
457+
queryParams(),
453458
session::headers,
454459
tableFileIO(context, response.config()),
455460
tableMetadata);
@@ -478,7 +483,7 @@ private MetricsReporter metricsReporter(
478483
String metricsEndpoint, Supplier<Map<String, String>> headers) {
479484
if (reportingViaRestEnabled) {
480485
RESTMetricsReporter restMetricsReporter =
481-
new RESTMetricsReporter(client, metricsEndpoint, headers);
486+
new RESTMetricsReporter(client, metricsEndpoint, headers, queryParams());
482487
return MetricsReporters.combine(reporter, restMetricsReporter);
483488
} else {
484489
return this.reporter;
@@ -510,10 +515,12 @@ public Table registerTable(
510515
.metadataLocation(metadataFileLocation)
511516
.build();
512517

518+
Map<String, String> queryParams = queryParams();
513519
LoadTableResponse response =
514520
client.post(
515521
paths.register(ident.namespace()),
516522
request,
523+
queryParams,
517524
LoadTableResponse.class,
518525
headers(context),
519526
ErrorHandlers.tableErrorHandler());
@@ -523,6 +530,7 @@ public Table registerTable(
523530
new RESTTableOperations(
524531
client,
525532
paths.table(ident),
533+
queryParams,
526534
session::headers,
527535
tableFileIO(context, response.config()),
528536
response.tableMetadata());
@@ -566,7 +574,7 @@ public List<Namespace> listNamespaces(SessionContext context, Namespace namespac
566574
ListNamespacesResponse response =
567575
client.get(
568576
paths.namespaces(),
569-
queryParams,
577+
queryParams(queryParams),
570578
ListNamespacesResponse.class,
571579
headers(context),
572580
ErrorHandlers.namespaceErrorHandler());
@@ -585,6 +593,7 @@ public Map<String, String> loadNamespaceMetadata(SessionContext context, Namespa
585593
GetNamespaceResponse response =
586594
client.get(
587595
paths.namespace(ns),
596+
queryParams(),
588597
GetNamespaceResponse.class,
589598
headers(context),
590599
ErrorHandlers.namespaceErrorHandler());
@@ -597,7 +606,11 @@ public boolean dropNamespace(SessionContext context, Namespace ns) {
597606

598607
try {
599608
client.delete(
600-
paths.namespace(ns), null, headers(context), ErrorHandlers.namespaceErrorHandler());
609+
paths.namespace(ns),
610+
queryParams(),
611+
null,
612+
headers(context),
613+
ErrorHandlers.namespaceErrorHandler());
601614
return true;
602615
} catch (NoSuchNamespaceException e) {
603616
return false;
@@ -616,6 +629,7 @@ public boolean updateNamespaceMetadata(
616629
client.post(
617630
paths.namespaceProperties(ns),
618631
request,
632+
queryParams(),
619633
UpdateNamespacePropertiesResponse.class,
620634
headers(context),
621635
ErrorHandlers.namespaceErrorHandler());
@@ -738,10 +752,12 @@ public Table create() {
738752
.setProperties(propertiesBuilder.build())
739753
.build();
740754

755+
Map<String, String> queryParams = queryParams();
741756
LoadTableResponse response =
742757
client.post(
743758
paths.tables(ident.namespace()),
744759
request,
760+
queryParams,
745761
LoadTableResponse.class,
746762
headers(context),
747763
ErrorHandlers.tableErrorHandler());
@@ -751,6 +767,7 @@ public Table create() {
751767
new RESTTableOperations(
752768
client,
753769
paths.table(ident),
770+
queryParams,
754771
session::headers,
755772
tableFileIO(context, response.config()),
756773
response.tableMetadata());
@@ -773,6 +790,7 @@ public Transaction createTransaction() {
773790
new RESTTableOperations(
774791
client,
775792
paths.table(ident),
793+
queryParams(),
776794
session::headers,
777795
tableFileIO(context, response.config()),
778796
RESTTableOperations.UpdateType.CREATE,
@@ -837,6 +855,7 @@ public Transaction replaceTransaction() {
837855
new RESTTableOperations(
838856
client,
839857
paths.table(ident),
858+
queryParams(),
840859
session::headers,
841860
tableFileIO(context, response.config()),
842861
RESTTableOperations.UpdateType.REPLACE,
@@ -881,12 +900,24 @@ private LoadTableResponse stageCreate() {
881900
return client.post(
882901
paths.tables(ident.namespace()),
883902
request,
903+
queryParams(),
884904
LoadTableResponse.class,
885905
headers(context),
886906
ErrorHandlers.tableErrorHandler());
887907
}
888908
}
889909

910+
private Map<String, String> queryParams() {
911+
return ImmutableMap.of("delim", namespaceSeparator);
912+
}
913+
914+
private Map<String, String> queryParams(Map<String, String> params) {
915+
return ImmutableMap.<String, String>builder()
916+
.putAll(params)
917+
.put("delim", namespaceSeparator)
918+
.build();
919+
}
920+
890921
private static List<MetadataUpdate> createChanges(TableMetadata meta) {
891922
ImmutableList.Builder<MetadataUpdate> changes = ImmutableList.builder();
892923

@@ -1137,7 +1168,7 @@ public List<TableIdentifier> listViews(SessionContext context, Namespace namespa
11371168
ListTablesResponse response =
11381169
client.get(
11391170
paths.views(namespace),
1140-
queryParams,
1171+
queryParams(queryParams),
11411172
ListTablesResponse.class,
11421173
headers(context),
11431174
ErrorHandlers.namespaceErrorHandler());
@@ -1152,11 +1183,13 @@ public List<TableIdentifier> listViews(SessionContext context, Namespace namespa
11521183
public View loadView(SessionContext context, TableIdentifier identifier) {
11531184
checkViewIdentifierIsValid(identifier);
11541185

1186+
Map<String, String> queryParams = queryParams();
11551187
LoadViewResponse response;
11561188
try {
11571189
response =
11581190
client.get(
11591191
paths.view(identifier),
1192+
queryParams,
11601193
LoadViewResponse.class,
11611194
headers(context),
11621195
ErrorHandlers.viewErrorHandler());
@@ -1173,7 +1206,8 @@ public View loadView(SessionContext context, TableIdentifier identifier) {
11731206
ViewMetadata metadata = response.metadata();
11741207

11751208
RESTViewOperations ops =
1176-
new RESTViewOperations(client, paths.view(identifier), session::headers, metadata);
1209+
new RESTViewOperations(
1210+
client, paths.view(identifier), queryParams, session::headers, metadata);
11771211

11781212
return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
11791213
}
@@ -1189,7 +1223,11 @@ public boolean dropView(SessionContext context, TableIdentifier identifier) {
11891223

11901224
try {
11911225
client.delete(
1192-
paths.view(identifier), null, headers(context), ErrorHandlers.viewErrorHandler());
1226+
paths.view(identifier),
1227+
queryParams(),
1228+
null,
1229+
headers(context),
1230+
ErrorHandlers.viewErrorHandler());
11931231
return true;
11941232
} catch (NoSuchViewException e) {
11951233
return false;
@@ -1295,18 +1333,20 @@ public View create() {
12951333
.properties(properties)
12961334
.build();
12971335

1336+
Map<String, String> queryParams = queryParams();
12981337
LoadViewResponse response =
12991338
client.post(
13001339
paths.views(identifier.namespace()),
13011340
request,
1341+
queryParams,
13021342
LoadViewResponse.class,
13031343
headers(context),
13041344
ErrorHandlers.viewErrorHandler());
13051345

13061346
AuthSession session = tableSession(response.config(), session(context));
13071347
RESTViewOperations ops =
13081348
new RESTViewOperations(
1309-
client, paths.view(identifier), session::headers, response.metadata());
1349+
client, paths.view(identifier), queryParams, session::headers, response.metadata());
13101350

13111351
return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
13121352
}
@@ -1332,6 +1372,7 @@ public View replace() {
13321372
private LoadViewResponse loadView() {
13331373
return client.get(
13341374
paths.view(identifier),
1375+
queryParams(),
13351376
LoadViewResponse.class,
13361377
headers(context),
13371378
ErrorHandlers.viewErrorHandler());
@@ -1376,7 +1417,8 @@ private View replace(LoadViewResponse response) {
13761417

13771418
AuthSession session = tableSession(response.config(), session(context));
13781419
RESTViewOperations ops =
1379-
new RESTViewOperations(client, paths.view(identifier), session::headers, metadata);
1420+
new RESTViewOperations(
1421+
client, paths.view(identifier), queryParams(), session::headers, metadata);
13801422

13811423
ops.commit(metadata, replacement);
13821424

‎core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ enum UpdateType {
5252

5353
private final RESTClient client;
5454
private final String path;
55+
private final Map<String, String> queryParams;
5556
private final Supplier<Map<String, String>> headers;
5657
private final FileIO io;
5758
private final List<MetadataUpdate> createChanges;
@@ -62,22 +63,25 @@ enum UpdateType {
6263
RESTTableOperations(
6364
RESTClient client,
6465
String path,
66+
Map<String, String> queryParams,
6567
Supplier<Map<String, String>> headers,
6668
FileIO io,
6769
TableMetadata current) {
68-
this(client, path, headers, io, UpdateType.SIMPLE, Lists.newArrayList(), current);
70+
this(client, path, queryParams, headers, io, UpdateType.SIMPLE, Lists.newArrayList(), current);
6971
}
7072

7173
RESTTableOperations(
7274
RESTClient client,
7375
String path,
76+
Map<String, String> queryParams,
7477
Supplier<Map<String, String>> headers,
7578
FileIO io,
7679
UpdateType updateType,
7780
List<MetadataUpdate> createChanges,
7881
TableMetadata current) {
7982
this.client = client;
8083
this.path = path;
84+
this.queryParams = queryParams;
8185
this.headers = headers;
8286
this.io = io;
8387
this.updateType = updateType;
@@ -149,7 +153,7 @@ public void commit(TableMetadata base, TableMetadata metadata) {
149153
// UnknownCommitStateException
150154
// TODO: ensure that the HTTP client lib passes HTTP client errors to the error handler
151155
LoadTableResponse response =
152-
client.post(path, request, LoadTableResponse.class, headers, errorHandler);
156+
client.post(path, request, queryParams, LoadTableResponse.class, headers, errorHandler);
153157

154158
// all future commits should be simple commits
155159
this.updateType = UpdateType.SIMPLE;

0 commit comments

Comments
 (0)