Skip to content

Commit

Permalink
fix(interactive): Fix Implicit Type Conversion Issues in HQPS (#3256)
Browse files Browse the repository at this point in the history
<!--
Thanks for your contribution! please review
https://github.com/alibaba/GraphScope/blob/main/CONTRIBUTING.md before
opening an issue.
-->

## What do these changes do?
Throw an error when the user input parameters are inconsistent with the
procedure config, to prevent the wrong type causing engine errors.

<!-- Please give a short brief about these changes. -->

## Related issue number

<!-- Are there any issues opened that will be resolved by merging this
change? -->

Fixes
  • Loading branch information
shirly121 authored Oct 10, 2023
1 parent b56948e commit d8b0185
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.*;
import org.apache.calcite.sql.SqlCallBinding;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.validate.implicit.TypeCoercion;
import org.apache.calcite.util.Litmus;

import java.util.Collection;
import java.util.List;
import java.util.function.Predicate;

Expand Down Expand Up @@ -159,7 +161,7 @@ private boolean checkSingleOperandType(
return true;
}

if (!family.getTypeNames().contains(typeName)) {
if (!getAllowedTypeNames(family, iFormalOperand).contains(typeName)) {
if (throwOnFailure) {
throw callBinding.newValidationSignatureError();
}
Expand All @@ -168,6 +170,11 @@ private boolean checkSingleOperandType(
return true;
}

protected Collection<SqlTypeName> getAllowedTypeNames(
SqlTypeFamily family, int iFormalOperand) {
return family.getTypeNames();
}

private boolean isNullLiteral(RexNode node) {
if (node instanceof RexLiteral) {
RexLiteral literal = (RexLiteral) node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,23 @@

package org.apache.calcite.sql.type;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;

import org.apache.calcite.linq4j.function.Functions;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlUtil;
import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;
import java.util.function.IntFunction;
import java.util.function.Predicate;
import java.util.stream.Collectors;

public class GraphOperandMetaDataImpl extends GraphFamilyOperandTypeChecker
implements SqlOperandMetadata {
Expand All @@ -33,26 +41,49 @@ public class GraphOperandMetaDataImpl extends GraphFamilyOperandTypeChecker

GraphOperandMetaDataImpl(
List<SqlTypeFamily> families,
Function<RelDataTypeFactory, List<RelDataType>> paramTypesFactory,
Function<@Nullable RelDataTypeFactory, List<RelDataType>> paramTypesFactory,
IntFunction<String> paramNameFn,
Predicate<Integer> optional) {
super(families, optional);
this.paramTypesFactory = Objects.requireNonNull(paramTypesFactory, "paramTypesFactory");
this.paramNameFn = paramNameFn;
}

@Override
protected Collection<SqlTypeName> getAllowedTypeNames(
SqlTypeFamily family, int iFormalOperand) {
List<RelDataType> paramsAllowedTypes = paramTypes(null);
Preconditions.checkArgument(
paramsAllowedTypes.size() > iFormalOperand,
"cannot find allowed type for type index="
+ iFormalOperand
+ " from the allowed types list="
+ paramsAllowedTypes);
return ImmutableList.of(paramsAllowedTypes.get(iFormalOperand).getSqlTypeName());
}

@Override
public boolean isFixedParameters() {
return true;
}

@Override
public List<RelDataType> paramTypes(RelDataTypeFactory typeFactory) {
return (List) this.paramTypesFactory.apply(typeFactory);
public List<RelDataType> paramTypes(@Nullable RelDataTypeFactory typeFactory) {
return this.paramTypesFactory.apply(typeFactory);
}

@Override
public List<String> paramNames() {
return Functions.generate(this.families.size(), this.paramNameFn);
}

@Override
public String getAllowedSignatures(SqlOperator op, String opName) {
return SqlUtil.getAliasedSignature(
op,
opName,
paramTypes(null).stream()
.map(k -> k.getSqlTypeName())
.collect(Collectors.toList()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,31 @@

import com.alibaba.graphscope.common.ir.tools.LogicalPlan;

import org.apache.calcite.runtime.CalciteException;
import org.junit.Assert;
import org.junit.Test;

public class CallProcedureTest {
@Test
public void match_1_test() {
public void procedure_1_test() {
LogicalPlan logicalPlan = Utils.evalLogicalPlan("Call ldbc_ic2(10l, 20120112l)");
Assert.assertEquals("ldbc_ic2(10:BIGINT, 20120112:BIGINT)", logicalPlan.explain().trim());
Assert.assertEquals(
"RecordType(CHAR(1) name)", logicalPlan.getProcedureCall().getType().toString());
}

// test procedure with invalid parameter types
@Test
public void procedure_2_test() {
try {
Utils.evalLogicalPlan("Call ldbc_ic2(10, 20120112l)");
} catch (CalciteException e) {
Assert.assertEquals(
"Cannot apply ldbc_ic2 to arguments of type 'ldbc_ic2(<INTEGER>, <BIGINT>)'."
+ " Supported form(s): 'ldbc_ic2(<BIGINT>, <BIGINT>)'",
e.getMessage());
return;
}
Assert.fail();
}
}

0 comments on commit d8b0185

Please sign in to comment.