Skip to content

Commit eb2f225

Browse files
committed
fix: review comments
Signed-off-by: MBWhite <whitemat@uk.ibm.com>
1 parent 2dec96d commit eb2f225

4 files changed

Lines changed: 8 additions & 7 deletions

File tree

isthmus/src/main/java/io/substrait/isthmus/expression/FunctionConverter.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ public FunctionConverter(
158158

159159
/**
160160
* Resolves a Calcite {@link SqlOperator} from a Substrait function key (Substrait → Calcite).
161+
* <p>
162+
* Given a Substrait function key (e.g., "concat:str_str") and output type, this method finds
163+
* the corresponding Calcite {@link SqlOperator}. When multiple operators match, the output type
164+
* is used to disambiguate.
161165
*
162166
* @param key Substrait function key (e.g., {@code concat:str_str})
163167
* @param outputType expected Substrait output type used for disambiguation

isthmus/src/main/java/io/substrait/isthmus/expression/FunctionMappings.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
* <p>Also provides type-based resolvers to disambiguate operators by output type.
1717
*/
1818
public class FunctionMappings {
19-
// Static list of signature mapping between Calcite SQL operators and Substrait base function
20-
// names.
2119

2220
/** Scalar operator signatures mapped to Substrait function names. */
2321
public static final ImmutableList<Sig> SCALAR_SIGS =
@@ -139,7 +137,7 @@ public class FunctionMappings {
139137
.addAll(AGGREGATE_SIGS)
140138
.build();
141139

142-
// contains return-type based resolver for both scalar and aggregator operator
140+
143141
/** Type-based resolvers to disambiguate Calcite operators by expected output type. */
144142
public static final Map<SqlOperator, TypeBasedResolver> OPERATOR_RESOLVER =
145143
Map.of(
@@ -169,7 +167,7 @@ public static void main(String[] args) {
169167
* Creates a signature mapping entry.
170168
*
171169
* @param operator the Calcite operator
172-
* @param substraitName the Substrait base function name
170+
* @param substraitName the Substrait canonical function name
173171
* @return a {@link Sig} instance
174172
*/
175173
public static Sig s(SqlOperator operator, String substraitName) {

isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,9 @@ public Boolean visit(Type.UUID type) {
221221
*
222222
* @param type user-defined type
223223
* @return {@code true} if {@code typeToMatch} equals {@code type}
224-
* @throws RuntimeException if comparison cannot be performed
225224
*/
226225
@Override
227-
public Boolean visit(Type.UserDefined type) throws RuntimeException {
226+
public Boolean visit(Type.UserDefined type) {
228227
// Two user-defined types are equal if they have the same uri AND name
229228
return typeToMatch.equals(type);
230229
}

isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlDialect.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public SubstraitSqlDialect(Context context) {
4949
/**
5050
* Indicates whether this dialect supports approximate COUNT(DISTINCT) operations.
5151
*
52-
* @return {@code true}, as Substrait SQL dialect supports approximate count distinct.
52+
* @return akways {@code true}, as Substrait SQL dialect supports approximate count distinct.
5353
*/
5454
@Override
5555
public boolean supportsApproxCountDistinct() {

0 commit comments

Comments
 (0)