Skip to content

Commit 433b045

Browse files
benbellickbenbellick
committed
feat(core): uri -> urn + add urn validation
This commit fully moves from URIs to URNs. This is meant as an intermediate commit in the graceful migration. Later work will re-add support for URIs in a way such that both URIs and URNs are accepted on parse and emitted in plans. It also adds necessary kotlin dep to make debugging work. BREAKING CHANGE: this commit alters the API by dropping support for URIs and adding support for URNs.
1 parent c490453 commit 433b045

27 files changed

Lines changed: 243 additions & 161 deletions

‎core/src/main/java/io/substrait/dsl/SubstraitBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ public Expression.WindowFunctionInvocation windowFn(
699699
// Types
700700

701701
public Type.UserDefined userDefinedType(String namespace, String typeName) {
702-
return Type.UserDefined.builder().uri(namespace).name(typeName).nullable(false).build();
702+
return Type.UserDefined.builder().urn(namespace).name(typeName).nullable(false).build();
703703
}
704704

705705
// Misc

‎core/src/main/java/io/substrait/expression/Expression.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -666,13 +666,13 @@ public <R, C extends VisitationContext, E extends Throwable> R accept(
666666
abstract class UserDefinedLiteral implements Literal {
667667
public abstract ByteString value();
668668

669-
public abstract String uri();
669+
public abstract String urn();
670670

671671
public abstract String name();
672672

673673
@Override
674674
public Type getType() {
675-
return Type.withNullability(nullable()).userDefined(uri(), name());
675+
return Type.withNullability(nullable()).userDefined(urn(), name());
676676
}
677677

678678
public static ImmutableExpression.UserDefinedLiteral.Builder builder() {

‎core/src/main/java/io/substrait/expression/ExpressionCreator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,10 @@ public static Expression.StructLiteral struct(
287287
}
288288

289289
public static Expression.UserDefinedLiteral userDefinedLiteral(
290-
boolean nullable, String uri, String name, Any value) {
290+
boolean nullable, String urn, String name, Any value) {
291291
return Expression.UserDefinedLiteral.builder()
292292
.nullable(nullable)
293-
.uri(uri)
293+
.urn(urn)
294294
.name(name)
295295
.value(value.toByteString())
296296
.build();

‎core/src/main/java/io/substrait/expression/proto/ExpressionProtoConverter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ public Expression visit(
361361
public Expression visit(
362362
io.substrait.expression.Expression.UserDefinedLiteral expr, EmptyVisitationContext context) {
363363
int typeReference =
364-
extensionCollector.getTypeReference(SimpleExtension.TypeAnchor.of(expr.uri(), expr.name()));
364+
extensionCollector.getTypeReference(SimpleExtension.TypeAnchor.of(expr.urn(), expr.name()));
365365
return lit(
366366
bldr -> {
367367
try {

‎core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) {
495495
SimpleExtension.Type type =
496496
lookup.getType(userDefinedLiteral.getTypeReference(), extensions);
497497
return ExpressionCreator.userDefinedLiteral(
498-
literal.getNullable(), type.uri(), type.name(), userDefinedLiteral.getValue());
498+
literal.getNullable(), type.urn(), type.name(), userDefinedLiteral.getValue());
499499
}
500500
default:
501501
throw new IllegalStateException("Unexpected value: " + literal.getLiteralTypeCase());

‎core/src/main/java/io/substrait/extension/DefaultExtensionCatalog.java‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
package io.substrait.extension;
22

33
public class DefaultExtensionCatalog {
4-
public static final String FUNCTIONS_AGGREGATE_APPROX = "/functions_aggregate_approx.yaml";
5-
public static final String FUNCTIONS_AGGREGATE_GENERIC = "/functions_aggregate_generic.yaml";
6-
public static final String FUNCTIONS_ARITHMETIC = "/functions_arithmetic.yaml";
7-
public static final String FUNCTIONS_ARITHMETIC_DECIMAL = "/functions_arithmetic_decimal.yaml";
8-
public static final String FUNCTIONS_BOOLEAN = "/functions_boolean.yaml";
9-
public static final String FUNCTIONS_COMPARISON = "/functions_comparison.yaml";
10-
public static final String FUNCTIONS_DATETIME = "/functions_datetime.yaml";
11-
public static final String FUNCTIONS_GEOMETRY = "/functions_geometry.yaml";
12-
public static final String FUNCTIONS_LOGARITHMIC = "/functions_logarithmic.yaml";
13-
public static final String FUNCTIONS_ROUNDING = "/functions_rounding.yaml";
14-
public static final String FUNCTIONS_ROUNDING_DECIMAL = "/functions_rounding_decimal.yaml";
15-
public static final String FUNCTIONS_SET = "/functions_set.yaml";
16-
public static final String FUNCTIONS_STRING = "/functions_string.yaml";
4+
public static final String FUNCTIONS_AGGREGATE_APPROX =
5+
"extension:io.substrait:functions_aggregate_approx";
6+
public static final String FUNCTIONS_AGGREGATE_GENERIC =
7+
"extension:io.substrait:functions_aggregate_generic";
8+
public static final String FUNCTIONS_ARITHMETIC = "extension:io.substrait:functions_arithmetic";
9+
public static final String FUNCTIONS_ARITHMETIC_DECIMAL =
10+
"extension:io.substrait:functions_arithmetic_decimal";
11+
public static final String FUNCTIONS_BOOLEAN = "extension:io.substrait:functions_boolean";
12+
public static final String FUNCTIONS_COMPARISON = "extension:io.substrait:functions_comparison";
13+
public static final String FUNCTIONS_DATETIME = "extension:io.substrait:functions_datetime";
14+
public static final String FUNCTIONS_GEOMETRY = "extension:io.substrait:functions_geometry";
15+
public static final String FUNCTIONS_LOGARITHMIC = "extension:io.substrait:functions_logarithmic";
16+
public static final String FUNCTIONS_ROUNDING = "extension:io.substrait:functions_rounding";
17+
public static final String FUNCTIONS_ROUNDING_DECIMAL =
18+
"extension:io.substrait:functions_rounding_decimal";
19+
public static final String FUNCTIONS_SET = "extension:io.substrait:functions_set";
20+
public static final String FUNCTIONS_STRING = "extension:io.substrait:functions_string";
1721
}

‎core/src/main/java/io/substrait/extension/ExtensionCollector.java‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import io.substrait.proto.ExtendedExpression;
44
import io.substrait.proto.Plan;
55
import io.substrait.proto.SimpleExtensionDeclaration;
6-
import io.substrait.proto.SimpleExtensionURI;
6+
import io.substrait.proto.SimpleExtensionURN;
77
import java.util.ArrayList;
88
import java.util.HashMap;
99
import java.util.Map;
@@ -52,71 +52,71 @@ public int getTypeReference(SimpleExtension.TypeAnchor typeAnchor) {
5252
public void addExtensionsToPlan(Plan.Builder builder) {
5353
SimpleExtensions simpleExtensions = getExtensions();
5454

55-
builder.addAllExtensionUris(simpleExtensions.uris.values());
55+
builder.addAllExtensionUrns(simpleExtensions.urns.values());
5656
builder.addAllExtensions(simpleExtensions.extensionList);
5757
}
5858

5959
public void addExtensionsToExtendedExpression(ExtendedExpression.Builder builder) {
6060
SimpleExtensions simpleExtensions = getExtensions();
6161

62-
builder.addAllExtensionUris(simpleExtensions.uris.values());
62+
builder.addAllExtensionUrns(simpleExtensions.urns.values());
6363
builder.addAllExtensions(simpleExtensions.extensionList);
6464
}
6565

6666
private SimpleExtensions getExtensions() {
67-
AtomicInteger uriPos = new AtomicInteger(1);
68-
HashMap<String, SimpleExtensionURI> uris = new HashMap<>();
67+
AtomicInteger urnPos = new AtomicInteger(1);
68+
HashMap<String, SimpleExtensionURN> urns = new HashMap<>();
6969

7070
ArrayList<SimpleExtensionDeclaration> extensionList = new ArrayList<>();
7171
for (Map.Entry<Integer, SimpleExtension.FunctionAnchor> e : funcMap.forwardMap.entrySet()) {
72-
SimpleExtensionURI uri =
73-
uris.computeIfAbsent(
74-
e.getValue().namespace(),
72+
SimpleExtensionURN urn =
73+
urns.computeIfAbsent(
74+
e.getValue().urn(),
7575
k ->
76-
SimpleExtensionURI.newBuilder()
77-
.setExtensionUriAnchor(uriPos.getAndIncrement())
78-
.setUri(k)
76+
SimpleExtensionURN.newBuilder()
77+
.setExtensionUrnAnchor(urnPos.getAndIncrement())
78+
.setUrn(k)
7979
.build());
8080
SimpleExtensionDeclaration decl =
8181
SimpleExtensionDeclaration.newBuilder()
8282
.setExtensionFunction(
8383
SimpleExtensionDeclaration.ExtensionFunction.newBuilder()
8484
.setFunctionAnchor(e.getKey())
8585
.setName(e.getValue().key())
86-
.setExtensionUriReference(uri.getExtensionUriAnchor()))
86+
.setExtensionUrnReference(urn.getExtensionUrnAnchor()))
8787
.build();
8888
extensionList.add(decl);
8989
}
9090
for (Map.Entry<Integer, SimpleExtension.TypeAnchor> e : typeMap.forwardMap.entrySet()) {
91-
SimpleExtensionURI uri =
92-
uris.computeIfAbsent(
93-
e.getValue().namespace(),
91+
SimpleExtensionURN urn =
92+
urns.computeIfAbsent(
93+
e.getValue().urn(),
9494
k ->
95-
SimpleExtensionURI.newBuilder()
96-
.setExtensionUriAnchor(uriPos.getAndIncrement())
97-
.setUri(k)
95+
SimpleExtensionURN.newBuilder()
96+
.setExtensionUrnAnchor(urnPos.getAndIncrement())
97+
.setUrn(k)
9898
.build());
9999
SimpleExtensionDeclaration decl =
100100
SimpleExtensionDeclaration.newBuilder()
101101
.setExtensionType(
102102
SimpleExtensionDeclaration.ExtensionType.newBuilder()
103103
.setTypeAnchor(e.getKey())
104104
.setName(e.getValue().key())
105-
.setExtensionUriReference(uri.getExtensionUriAnchor()))
105+
.setExtensionUrnReference(urn.getExtensionUrnAnchor()))
106106
.build();
107107
extensionList.add(decl);
108108
}
109-
return new SimpleExtensions(uris, extensionList);
109+
return new SimpleExtensions(urns, extensionList);
110110
}
111111

112112
private static final class SimpleExtensions {
113-
final HashMap<String, SimpleExtensionURI> uris;
113+
final HashMap<String, SimpleExtensionURN> urns;
114114
final ArrayList<SimpleExtensionDeclaration> extensionList;
115115

116116
SimpleExtensions(
117-
HashMap<String, SimpleExtensionURI> uris,
117+
HashMap<String, SimpleExtensionURN> urns,
118118
ArrayList<SimpleExtensionDeclaration> extensionList) {
119-
this.uris = uris;
119+
this.urns = urns;
120120
this.extensionList = extensionList;
121121
}
122122
}

‎core/src/main/java/io/substrait/extension/ImmutableExtensionLookup.java‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import io.substrait.proto.ExtendedExpression;
44
import io.substrait.proto.Plan;
55
import io.substrait.proto.SimpleExtensionDeclaration;
6-
import io.substrait.proto.SimpleExtensionURI;
6+
import io.substrait.proto.SimpleExtensionURN;
77
import java.util.Collections;
88
import java.util.HashMap;
99
import java.util.List;
@@ -30,20 +30,21 @@ public static class Builder {
3030
private final Map<Integer, SimpleExtension.TypeAnchor> typeMap = new HashMap<>();
3131

3232
public Builder from(Plan plan) {
33-
return from(plan.getExtensionUrisList(), plan.getExtensionsList());
33+
return from(plan.getExtensionUrnsList(), plan.getExtensionsList());
3434
}
3535

3636
public Builder from(ExtendedExpression extendedExpression) {
3737
return from(
38-
extendedExpression.getExtensionUrisList(), extendedExpression.getExtensionsList());
38+
extendedExpression.getExtensionUrnsList(), extendedExpression.getExtensionsList());
3939
}
4040

4141
private Builder from(
42-
List<SimpleExtensionURI> simpleExtensionURIs,
42+
List<SimpleExtensionURN> simpleExtensionURNs,
4343
List<SimpleExtensionDeclaration> simpleExtensionDeclarations) {
44-
Map<Integer, String> namespaceMap = new HashMap<>();
45-
for (SimpleExtensionURI extension : simpleExtensionURIs) {
46-
namespaceMap.put(extension.getExtensionUriAnchor(), extension.getUri());
44+
Map<Integer, String> urnMap = new HashMap<>();
45+
// Handle URN format
46+
for (SimpleExtensionURN extension : simpleExtensionURNs) {
47+
urnMap.put(extension.getExtensionUrnAnchor(), extension.getUrn());
4748
}
4849

4950
// Add all functions used in plan to the functionMap
@@ -53,13 +54,14 @@ private Builder from(
5354
}
5455
SimpleExtensionDeclaration.ExtensionFunction func = extension.getExtensionFunction();
5556
int reference = func.getFunctionAnchor();
56-
String namespace = namespaceMap.get(func.getExtensionUriReference());
57-
if (namespace == null) {
57+
String urn = urnMap.get(func.getExtensionUrnReference());
58+
if (urn == null) {
5859
throw new IllegalStateException(
59-
"Could not find extension URI of " + func.getExtensionUriReference());
60+
"Could not find extension URN for function reference "
61+
+ func.getExtensionUrnReference());
6062
}
6163
String name = func.getName();
62-
SimpleExtension.FunctionAnchor anchor = SimpleExtension.FunctionAnchor.of(namespace, name);
64+
SimpleExtension.FunctionAnchor anchor = SimpleExtension.FunctionAnchor.of(urn, name);
6365
functionMap.put(reference, anchor);
6466
}
6567

@@ -70,13 +72,13 @@ private Builder from(
7072
}
7173
SimpleExtensionDeclaration.ExtensionType type = extension.getExtensionType();
7274
int reference = type.getTypeAnchor();
73-
String namespace = namespaceMap.get(type.getExtensionUriReference());
74-
if (namespace == null) {
75+
String urn = urnMap.get(type.getExtensionUrnReference());
76+
if (urn == null) {
7577
throw new IllegalStateException(
76-
"Could not find extension URI of " + type.getExtensionUriReference());
78+
"Could not find extension URN for type reference " + type.getExtensionUrnReference());
7779
}
7880
String name = type.getName();
79-
SimpleExtension.TypeAnchor anchor = SimpleExtension.TypeAnchor.of(namespace, name);
81+
SimpleExtension.TypeAnchor anchor = SimpleExtension.TypeAnchor.of(urn, name);
8082
typeMap.put(reference, anchor);
8183
}
8284

0 commit comments

Comments
 (0)