Skip to content

Commit

Permalink
fix(interactive): Fix bugs of Chinese Encoding (#3195)
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?
Addressing the Chinese character encoding issue in the Calcite, the
Gremlin is normal as it has not been integrated into Calcite yet.

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

## Related issue number
fix #3146 
<!-- Are there any issues opened that will be resolved by merging this
change? -->

Fixes

---------

Co-authored-by: Longbin Lai <[email protected]>
  • Loading branch information
shirly121 and longbinlai authored Sep 8, 2023
1 parent f08489e commit d4d5506
Show file tree
Hide file tree
Showing 15 changed files with 99 additions and 23 deletions.
2 changes: 2 additions & 0 deletions interactive_engine/compiler/conf/ir.compiler.properties
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,5 @@ graph.planner.rules: FilterIntoJoinRule, FilterMatchRule, NotMatchToAntiJoinRule

# set timeout in system config, can be overridden by session config per query
# query.execution.timeout.ms: 3000000

calcite.default.charset: UTF-8
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,7 @@ public class FrontendConfig {

public static final Config<Integer> FRONTEND_SERVER_NUM =
Config.intConfig("frontend.server.num", 1);

public static final Config<String> CALCITE_DEFAULT_CHARSET =
Config.stringConfig("calcite.default.charset", "UTF-8");
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ public class YamlConfigs extends Configs {
.put(
"query.execution.timeout.ms",
(Configs configs) -> configs.get("compiler.query_timeout"))
.put("engine.type", (Configs configs) -> configs.get("compute_engine.type"));
.put("engine.type", (Configs configs) -> configs.get("compute_engine.type"))
.put(
"calcite.default.charset",
(Configs configs) -> configs.get("compiler.calcite_default_charset"));
valueGetterMap = mapBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public GraphPlanner(Configs graphConfig) {
this.plannerConfig = PlannerConfig.create(this.graphConfig);
logger.debug("planner config: " + this.plannerConfig);
this.optPlanner = createRelOptPlanner(this.plannerConfig);
this.rexBuilder = new GraphRexBuilder(new GraphTypeFactoryImpl());
this.rexBuilder = new GraphRexBuilder(new GraphTypeFactoryImpl(graphConfig));
this.idGenerator = new AtomicLong(FrontendConfig.FRONTEND_SERVER_ID.get(graphConfig));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,22 @@

package com.alibaba.graphscope.common.ir.type;

import com.alibaba.graphscope.common.config.Configs;
import com.alibaba.graphscope.common.config.FrontendConfig;
import com.google.common.collect.Lists;

import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
import org.apache.calcite.rel.type.RelDataType;

import java.nio.charset.Charset;

public class GraphTypeFactoryImpl extends JavaTypeFactoryImpl {
private final Configs configs;

public GraphTypeFactoryImpl(Configs configs) {
super();
this.configs = configs;
}

@Override
public RelDataType createTypeWithNullability(RelDataType type, boolean nullable) {
Expand All @@ -44,4 +54,9 @@ public RelDataType createTypeWithNullability(RelDataType type, boolean nullable)
}
return newType;
}

@Override
public Charset getDefaultCharset() {
return Charset.forName(FrontendConfig.CALCITE_DEFAULT_CHARSET.get(configs));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,11 @@ public void schema_config_test() throws Exception {
+ " DefaultGraphProperty{id=2, name=age, dataType=INT}], primaryKeyList=[id]}",
graphSchema.getElement("person").toString());
}

@Test
public void compiler_config_test() throws Exception {
YamlConfigs configs =
new YamlConfigs("config/gs_interactive_hiactor.yaml", FileLoadType.RESOURCES);
Assert.assertEquals("UTF-8", FrontendConfig.CALCITE_DEFAULT_CHARSET.get(configs));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void case_1_test() {
builder.literal("marko")),
builder.literal(1),
builder.literal(3));
Assert.assertEquals("CASE(=(a.name, 'marko'), 1, 3)", caseExpr.toString());
Assert.assertEquals("CASE(=(a.name, _UTF-8'marko'), 1, 3)", caseExpr.toString());
Assert.assertEquals(SqlTypeName.INTEGER, caseExpr.getType().getSqlTypeName());
}

Expand All @@ -73,7 +73,7 @@ public void case_2_test() {
builder.literal("marko")),
rexBuilder.makeGraphDynamicParam("value", 0),
builder.literal(3));
Assert.assertEquals("CASE(=(a.name, 'marko'), ?0, 3)", caseExpr.toString());
Assert.assertEquals("CASE(=(a.name, _UTF-8'marko'), ?0, 3)", caseExpr.toString());
Assert.assertEquals(SqlTypeName.INTEGER, caseExpr.getType().getSqlTypeName());
Assert.assertEquals(
SqlTypeName.INTEGER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void equal_2_test() {
RexNode var = builder.source(mockSourceConfig("a")).variable("a", "age");
RexNode equal = builder.call(GraphStdOperatorTable.EQUALS, var, builder.literal("X"));
Assert.assertEquals(equal.getType().getSqlTypeName(), SqlTypeName.BOOLEAN);
Assert.assertEquals("=(a.age, 'X')", equal.toString());
Assert.assertEquals("=(a.age, _UTF-8'X')", equal.toString());
}

// a.age + 10 == 30
Expand All @@ -138,7 +138,7 @@ public void and_test() {
RexNode equal2 = builder.call(GraphStdOperatorTable.EQUALS, var2, builder.literal("x"));
RexNode node = builder.call(GraphStdOperatorTable.AND, equal1, equal2);
Assert.assertEquals(node.getType().getSqlTypeName(), SqlTypeName.BOOLEAN);
Assert.assertEquals("AND(>(a.age, 10), =(a.name, 'x'))", node.toString());
Assert.assertEquals("AND(>(a.age, 10), =(a.name, _UTF-8'x'))", node.toString());
}

// a.age % 10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ public void and_1_test() {
RelNode filter = builder.filter(condition1, condition2).build();
Assert.assertEquals(
"GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}], alias=[DEFAULT],"
+ " fusedFilter=[[AND(>(DEFAULT.age, 20), =(DEFAULT.name, 'marko'))]],"
+ " opt=[VERTEX])",
+ " fusedFilter=[[AND(>(DEFAULT.age, 20), =(DEFAULT.name, _UTF-8'marko'))]],"
+ " opt=[VERTEX])",
filter.explain().trim());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.alibaba.graphscope.common.ir;

import com.alibaba.graphscope.common.config.Configs;
import com.alibaba.graphscope.common.config.FrontendConfig;
import com.alibaba.graphscope.common.config.GraphConfig;
import com.alibaba.graphscope.common.ir.meta.reader.LocalMetaDataReader;
import com.alibaba.graphscope.common.ir.meta.schema.GraphOptSchema;
Expand All @@ -39,7 +40,11 @@
import java.net.URL;

public class Utils {
public static final RelDataTypeFactory typeFactory = new GraphTypeFactoryImpl();
public static final RelDataTypeFactory typeFactory =
new GraphTypeFactoryImpl(
new Configs(
ImmutableMap.of(
FrontendConfig.CALCITE_DEFAULT_CHARSET.getKey(), "UTF-8")));
public static final RexBuilder rexBuilder = new GraphRexBuilder(typeFactory);
public static final IrMeta schemaMeta = mockSchemaMeta();
public static final RelBuilderFactory relBuilderFactory =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ public void logical_plan_5_test() throws Exception {
RelNode project =
builder.project(ImmutableList.of(caseExpr), ImmutableList.of("case")).build();
Assert.assertEquals(
"GraphLogicalProject(case=[CASE(=(a.name, 'marko'), 1, 3)], isAppend=[false])\n"
"GraphLogicalProject(case=[CASE(=(a.name, _UTF-8'marko'), 1, 3)],"
+ " isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], opt=[VERTEX])",
project.explain().trim());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.calcite.plan.RelOptPlanner;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rex.RexCall;
import org.apache.calcite.runtime.CalciteException;
import org.apache.calcite.sql.type.SqlTypeName;
import org.junit.Assert;
import org.junit.Test;
Expand Down Expand Up @@ -93,7 +94,7 @@ public void match_4_test() {
"GraphLogicalProject(a=[a], b=[b], isAppend=[false])\n"
+ " GraphLogicalSingleMatch(input=[null],"
+ " sentence=[GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[c], fusedFilter=[[=(DEFAULT.name, 'marko')]], opt=[END])\n"
+ " alias=[c], fusedFilter=[[=(DEFAULT.name, _UTF-8'marko')]], opt=[END])\n"
+ " GraphLogicalPathExpand(expand=[GraphLogicalExpand(tableConfig=[{isAll=false,"
+ " tables=[knows]}], alias=[DEFAULT], fusedFilter=[[=(DEFAULT.weight,"
+ " 1.0E0)]], opt=[OUT])\n"
Expand Down Expand Up @@ -237,4 +238,41 @@ public void match_11_test() {
plan.getDynamicParams().toString());
Assert.assertEquals("RecordType(BIGINT id, CHAR(1) name)", plan.getOutputType().toString());
}

@Test
public void match_12_test() {
try {
RelNode node = Utils.eval("Match (a:人类) Return a").build();
} catch (CalciteException e) {
Assert.assertEquals("Table '人类' not found", e.getMessage());
return;
}
Assert.fail();
}

@Test
public void match_13_test() {
try {
RelNode node = Utils.eval("Match (a:person {名称:'marko'}) Return a").build();
} catch (IllegalArgumentException e) {
Assert.assertEquals(
"{property=名称} not found; expected properties are: [id, name, age]",
e.getMessage());
return;
}
Assert.fail();
}

@Test
public void match_14_test() {
RelNode node = Utils.eval("Match (a:person {name:'小明'}) Return '小明'").build();
Assert.assertEquals(
"GraphLogicalProject($f0=[_UTF-8'小明'], isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], fusedFilter=[[=(DEFAULT.name, _UTF-8'小明')]], opt=[VERTEX])",
node.explain().trim());
Assert.assertEquals(
SqlTypeName.CHAR,
node.getRowType().getFieldList().get(0).getType().getSqlTypeName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public void where_1_test() {
.build();
Assert.assertEquals(
"GraphLogicalProject(a=[a], b=[b], c=[c], isAppend=[false])\n"
+ " LogicalFilter(condition=[OR(AND(=(a.name, 'marko'), <(b.weight, 2.0E0)),"
+ " <(+(c.age, 10), a.age))])\n"
+ " LogicalFilter(condition=[OR(AND(=(a.name, _UTF-8'marko'), <(b.weight,"
+ " 2.0E0)), <(+(c.age, 10), a.age))])\n"
+ " GraphLogicalSingleMatch(input=[null],"
+ " sentence=[GraphLogicalGetV(tableConfig=[{isAll=true, tables=[software,"
+ " person]}], alias=[c], opt=[END])\n"
Expand All @@ -59,8 +59,9 @@ public void where_2_test() {
Assert.assertEquals(
"GraphLogicalProject(a=[a], isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=true, tables=[software, person]}],"
+ " alias=[a], fusedFilter=[[AND(=(DEFAULT.name, 'kli'), OR(=(+(DEFAULT.age,"
+ " 1), 29), =(DEFAULT.name, 'marko')))]], opt=[VERTEX])",
+ " alias=[a], fusedFilter=[[AND(=(DEFAULT.name, _UTF-8'kli'),"
+ " OR(=(+(DEFAULT.age, 1), 29), =(DEFAULT.name, _UTF-8'marko')))]],"
+ " opt=[VERTEX])",
where.explain().trim());
}

Expand Down Expand Up @@ -107,9 +108,9 @@ public void where_5_test() {
.build();
Assert.assertEquals(
"GraphLogicalProject(a=[a], isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], fusedFilter=[[>(CASE(=(DEFAULT.name, 'marko'), 1, >(DEFAULT.age,"
+ " 10), 2, 3), 2)]], opt=[VERTEX])",
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], fusedFilter=[[>(CASE(=(DEFAULT.name, _UTF-8'marko'), 1,"
+ " >(DEFAULT.age, 10), 2, 3), 2)]], opt=[VERTEX])",
where.explain().trim());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ public void with_7_test() {
+ " 'vadas' THEN 2 ELSE 3 END as d")
.build();
Assert.assertEquals(
"GraphLogicalProject(d=[CASE(=(a.name, 'marko'), 1, =(a.name, 'vadas'), 2, 3)],"
+ " isAppend=[false])\n"
"GraphLogicalProject(d=[CASE(=(a.name, _UTF-8'marko'), 1, =(a.name, _UTF-8'vadas'),"
+ " 2, 3)], isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], opt=[VERTEX])",
project.explain().trim());
Expand All @@ -131,7 +131,7 @@ public void with_8_test() {
+ " a.age > 10 THEN 2 ELSE 3 END as d")
.build();
Assert.assertEquals(
"GraphLogicalProject(d=[CASE(=(a.name, 'marko'), 1, >(a.age, 10), 2, 3)],"
"GraphLogicalProject(d=[CASE(=(a.name, _UTF-8'marko'), 1, >(a.age, 10), 2, 3)],"
+ " isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], opt=[VERTEX])",
Expand All @@ -147,8 +147,8 @@ public void with_9_test() {
+ " 'josh' THEN 2 END as d")
.build();
Assert.assertEquals(
"GraphLogicalProject(d=[CASE(=(a.name, 'marko'), 1, =(a.name, 'josh'), 2,"
+ " null:NULL)], isAppend=[false])\n"
"GraphLogicalProject(d=[CASE(=(a.name, _UTF-8'marko'), 1, =(a.name, _UTF-8'josh'),"
+ " 2, null:NULL)], isAppend=[false])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], opt=[VERTEX])",
project.explain().trim());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ compiler:
disabled: false # default false
port: 8003 # default 7687
query_timeout: 200 # query timeout in milliseconds, default 2000
calcite_default_charset: UTF-8

0 comments on commit d4d5506

Please sign in to comment.