Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -255,13 +255,19 @@ public static RelDataType toSql(final RelDataTypeFactory typeFactory,
SqlTypeName sqlTypeName = type.getSqlTypeName();
final RelDataType relDataType;
if (SqlTypeUtil.isArray(type)) {
final RelDataType elementType = type.getComponentType() == null
// type.getJavaClass() is collection with erased generic type
? typeFactory.createSqlType(SqlTypeName.ANY)
// elementType returned by JavaType is also of JavaType,
// and needs conversion using typeFactory
: toSql(typeFactory, type.getComponentType());
// Transform to sql type, take care for two cases:
// 1. type.getJavaClass() is collection with erased generic type
// 2. ElementType returned by JavaType is also of JavaType,
// and needs conversion using typeFactory
final RelDataType elementType = toSqlTypeWithNullToAny(
typeFactory, type.getComponentType());
relDataType = typeFactory.createArrayType(elementType, -1);
} else if (SqlTypeUtil.isMap(type)) {
final RelDataType keyType = toSqlTypeWithNullToAny(
typeFactory, type.getKeyType());
final RelDataType valueType = toSqlTypeWithNullToAny(
typeFactory, type.getValueType());
relDataType = typeFactory.createMapType(keyType, valueType);
} else {
relDataType = typeFactory.createSqlType(sqlTypeName);
}
Expand All @@ -270,6 +276,14 @@ public static RelDataType toSql(final RelDataTypeFactory typeFactory,
return type;
}

private static RelDataType toSqlTypeWithNullToAny(
final RelDataTypeFactory typeFactory, RelDataType type) {
if (type == null) {
return typeFactory.createSqlType(SqlTypeName.ANY);
}
return toSql(typeFactory, type);
}

public Type createSyntheticType(List<Type> types) {
if (types.isEmpty()) {
// Unit is a pre-defined synthetic type to be used when there are 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,32 @@ public RelDataType getComponentType() {
}
}

/**
* For {@link JavaType} created with {@link Map} class,
* cannot get the type of key and value.
* Using ANY type as key type.
*/
@Override public RelDataType getKeyType() {
if (Map.class.isAssignableFrom(clazz)) {
return createSqlType(SqlTypeName.ANY);
Copy link
Contributor

@jinxing64 jinxing64 Oct 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say the map is by Integer -> Integer
Why return SqlTypeName.ANY here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I cannot find a way to get the Integer parameter type when create type with Map class.
And use SqlTypeName.ANY can cover other types.

} else {
return null;
}
}

/**
* For {@link JavaType} created with {@link Map} class,
* cannot get the type of key and value.
* Using ANY type as value type.
*/
@Override public RelDataType getValueType() {
if (Map.class.isAssignableFrom(clazz)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does any code use these 2 methods ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, inSqlItemOperator.
If don't override these 2 methods, running org.apache.calcite.adapter.geode.rel.GeodeBookstoreTest will get NullPointerException

Caused by: java.lang.NullPointerException
	at org.apache.calcite.sql.fun.SqlItemOperator.getChecker(SqlItemOperator.java:103)
	at org.apache.calcite.sql.fun.SqlItemOperator.checkOperandTypes(SqlItemOperator.java:91)
	at org.apache.calcite.sql.SqlOperator.validateOperands(SqlOperator.java:432)
	at org.apache.calcite.sql.SqlOperator.deriveType(SqlOperator.java:518)
	at org.apache.calcite.sql.validate.SqlValidatorImpl$DeriveTypeVisitor.visit(SqlValidatorImpl.java:5639)
	at org.apache.calcite.sql.validate.SqlValidatorImpl$DeriveTypeVisitor.visit(SqlValidatorImpl.java:5626)
	at org.apache.calcite.sql.SqlCall.accept(SqlCall.java:139)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.deriveTypeImpl(SqlValidatorImpl.java:1692)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.deriveType(SqlValidatorImpl.java:1677)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.expandSelectItem(SqlValidatorImpl.java:480)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelectList(SqlValidatorImpl.java:4117)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelect(SqlValidatorImpl.java:3396)
	at org.apache.calcite.sql.validate.SelectNamespace.validateImpl(SelectNamespace.java:60)
	at org.apache.calcite.sql.validate.AbstractNamespace.validate(AbstractNamespace.java:84)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.validateNamespace(SqlValidatorImpl.java:1009)
	at org.apache.calcite.sql.validate.SqlValidatorImpl.validateQuery(SqlValidatorImpl.java:969)
	at org.apache.calcite.sql.SqlSelect.validate(SqlSelect.java:216)

return createSqlType(SqlTypeName.ANY);
} else {
return null;
}
}

public Charset getCharset() {
return this.charset;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public class JavaToSqlTypeConversionRules {
.put(ColumnList.class, SqlTypeName.COLUMN_LIST)
.put(ArrayImpl.class, SqlTypeName.ARRAY)
.put(List.class, SqlTypeName.ARRAY)
.put(Map.class, SqlTypeName.MAP)
.put(Void.class, SqlTypeName.NULL)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ private SqlTypeAssignmentRules(
// ARRAY is assignable from ...
rules.add(SqlTypeName.ARRAY, EnumSet.of(SqlTypeName.ARRAY));

// Map is assignable from ...
rules.add(SqlTypeName.MAP, EnumSet.of(SqlTypeName.MAP));

// ANY is assignable from ...
rule.clear();
rule.add(SqlTypeName.TINYINT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ private void assertBasic(SqlTypeName typeName) {
: "use createMultisetType() instead";
assert typeName != SqlTypeName.ARRAY
: "use createArrayType() instead";
assert typeName != SqlTypeName.MAP
: "use createMapType() instead";
assert typeName != SqlTypeName.ROW
: "use createStructType() instead";
assert !SqlTypeName.INTERVAL_TYPES.contains(typeName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -153,6 +154,23 @@ public void createStructTypeWithNullability() {
assertTrue(copyRecordType.isNullable());
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-3429">[CALCITE-3429]
* AssertionError thrown for user-defined table function with map argument</a>. */
@Test public void testCreateTypeWithJavaMapType() {
SqlTypeFixture f = new SqlTypeFixture();
RelDataType relDataType = f.typeFactory.createJavaType(Map.class);
assertThat(relDataType.getSqlTypeName(), is(SqlTypeName.MAP));
assertThat(relDataType.getKeyType().getSqlTypeName(), is(SqlTypeName.ANY));

try {
f.typeFactory.createSqlType(SqlTypeName.MAP);
fail();
} catch (AssertionError e) {
assertThat(e.getMessage(), is("use createMapType() instead"));
}
}

}

// End SqlTypeFactoryTest.java
18 changes: 18 additions & 0 deletions core/src/test/java/org/apache/calcite/test/TableFunctionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,24 @@ private CalciteAssert.AssertThat with() {
}
}

@Test public void testTableFunctionWithMapParameter() throws SQLException {
try (Connection connection = DriverManager.getConnection("jdbc:calcite:")) {
CalciteConnection calciteConnection =
connection.unwrap(CalciteConnection.class);
SchemaPlus rootSchema = calciteConnection.getRootSchema();
SchemaPlus schema = rootSchema.add("s", new AbstractSchema());
final TableFunction table =
TableFunctionImpl.create(Smalls.GENERATE_STRINGS_OF_INPUT_MAP_SIZE_METHOD);
schema.add("GenerateStringsOfInputMapSize", table);
final String sql = "select *\n"
+ "from table(\"s\".\"GenerateStringsOfInputMapSize\"(Map[5,4,3,1])) as t(n, c)\n"
+ "where char_length(c) > 0";
ResultSet resultSet = connection.createStatement().executeQuery(sql);
assertThat(CalciteAssert.toString(resultSet),
equalTo("N=1; C=a\n"));
}
}

/**
* Tests a table function that implements {@link ScannableTable} and returns
* a single column.
Expand Down
6 changes: 6 additions & 0 deletions core/src/test/java/org/apache/calcite/util/Smalls.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;

import static org.hamcrest.CoreMatchers.is;
Expand All @@ -74,6 +75,8 @@ public class Smalls {
Types.lookupMethod(Smalls.class, "generateStrings", Integer.class);
public static final Method GENERATE_STRINGS_OF_INPUT_SIZE_METHOD =
Types.lookupMethod(Smalls.class, "generateStringsOfInputSize", List.class);
public static final Method GENERATE_STRINGS_OF_INPUT_MAP_SIZE_METHOD =
Types.lookupMethod(Smalls.class, "generateStringsOfInputMapSize", Map.class);
public static final Method MAZE_METHOD =
Types.lookupMethod(MazeTable.class, "generate", int.class, int.class,
int.class);
Expand Down Expand Up @@ -187,6 +190,9 @@ public void close() {
public static QueryableTable generateStringsOfInputSize(final List<Integer> list) {
return generateStrings(list.size());
}
public static QueryableTable generateStringsOfInputMapSize(final Map<Integer, Integer> map) {
return generateStrings(map.size());
}

/** A function that generates multiplication table of {@code ncol} columns x
* {@code nrow} rows. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ public Enumerable<Object> query(final GemFireCache clientCache,
type = typeFactory.createMultisetType(
typeFactory.createSqlType(SqlTypeName.ANY),
-1);
} else if (typeName == SqlTypeName.MAP) {
RelDataType anyType = typeFactory.createSqlType(SqlTypeName.ANY);
type = typeFactory.createMapType(anyType, anyType);
} else {
type = typeFactory.createSqlType(typeName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,20 @@

import org.apache.geode.cache.Cache;
import org.apache.geode.cache.Region;
import org.apache.geode.pdx.PdxInstance;

import org.junit.BeforeClass;
import org.junit.Test;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.function.Consumer;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

/**
* Tests using {@code Bookshop} schema.
Expand Down Expand Up @@ -379,10 +385,37 @@ public void testSelectWithNestedPdx2() {
calciteAssert()
.query("select primaryAddress from geode.BookCustomer limit 2")
.returnsCount(2)
.returns("primaryAddress=PDX[addressLine1,addressLine2,addressLine3,city,state,"
+ "postalCode,country,phoneNumber,addressTag]\n"
+ "primaryAddress=PDX[addressLine1,addressLine2,addressLine3,city,state,postalCode,"
+ "country,phoneNumber,addressTag]\n")
.returns(new Consumer<ResultSet>() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I update this case to make it pass.
But I'm not quite familiar with the logic, hopes someone more familiar with it can review this.

/**
* The result of {@link org.apache.geode.pdx.internal.PdxInstanceImpl}
* instance's toString method, has a prefix with random int value,
* removes the prefix before checking. The prefix may like this
* "PDX[10872742,org.apache.calcite.adapter.geode.primaryAddress]"
*/
@Override public void accept(ResultSet resultSet) {
try {
StringBuilder builder = new StringBuilder();
final String prefix = "org.apache.calcite.adapter.geode.primaryAddress";
while (resultSet.next()) {
PdxInstance o = (PdxInstance) resultSet.getObject(1);
int index = o.toString().indexOf(prefix);
builder.append(
o.toString().substring(index + prefix.length() + 1));
builder.append("\n");
}
assertThat(builder.toString(),
is("{addressLine1=123 Main St., addressLine2=java.lang.NullPointerException,"
+ " addressLine3=java.lang.NullPointerException, addressTag=HOME, city=Topeka,"
+ " country=US, phoneNumber=423-555-3322, postalCode=50505, state=KS}\n"
+ "{addressLine1=123 Main St., addressLine2=java.lang.NullPointerException,"
+ " addressLine3=java.lang.NullPointerException, addressTag=HOME,"
+ " city=San Francisco, country=US," + " phoneNumber=423-555-3322,"
+ " postalCode=50505, state=CA}\n"));
} catch (Exception e) {
throw new RuntimeException(e);
}
}
})
.explainContains("PLAN=GeodeToEnumerableConverter\n"
+ " GeodeProject(primaryAddress=[$3])\n"
+ " GeodeSort(fetch=[2])\n"
Expand Down