-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recursive Calcite sub-schema support #147
Conversation
Doesn't work currently, but also doesn't break existing code I'm not sure how to create a new ML relation type and attach the results nested under that, but this is a decent scaffold of the logic (I hope)
for (String subSchemaName : subSchemaNames) { | ||
final SchemaPlus subSchema = requireNonNull(currentSchema.getSubSchema(subSchemaName)); | ||
System.out.println("CalciteForeignValue - processing sub-schema: " + subSchemaName); | ||
// TODO: How to create a new ML record type for each sub-schema? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to create a record type, and an instance of it.
TypeSystem.recordType(SortedMap)
creates the type. One field per sub-schema (or table) in the schema.
A record value is just a Java List. The entries correspond to the fields (which are always sorted by name). You should wrap each Schema
object as a CalciteForeignValue
, and each Table
object as a RelList
. (CalciteForeignValue.value()
seems to do a lot of this already.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want an example of code creating an instance of a record, see how Codes.TupleCode.eval(EvalEnv)
calls Arrays.asList
. Tuples and records are the same at runtime (the only difference is that tuples have field names "1", "2", "3" etc whereas records have alphabetical names) so this piece of code is used for both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay great, thank you! Will try to implement this based on your info here 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: Okay I've gotten really close, it looks like the sub-schemas are just double nested in the Map
.
Recursive logic hurts my head, trying to figure out the right way to rewrite this so that it's pulled up a level 😅
schema: users
subSchema: todos
schema: todos
fieldMap = {todos={completed:bool, name:string} list}
fieldMap = {todos={todos:{completed:bool, name:string} list}, users={age:int, name:string} list}
[Out]: java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.util.List (java.lang.Integer and java.util.List are in module java.base of loader 'bootstrap')
public Type type(TypeSystem typeSystem) {
return type(schema, typeSystem);
}
public Type type(SchemaPlus schema, TypeSystem typeSystem) {
final ImmutableSortedMap.Builder<String, Type> fields =
ImmutableSortedMap.orderedBy(RecordType.ORDERING);
System.out.println("schema: " + schema.getName());
schema.getTableNames().forEach(tableName ->
fields.put(convert(tableName),
toType(schema.getTable(tableName), typeSystem)));
schema.getSubSchemaNames().forEach(subSchemaName -> {
final SchemaPlus subSchema = schema.getSubSchema(subSchemaName);
System.out.println("subSchema: " + subSchema.getName());
fields.put(convert(subSchemaName),
type(subSchema, typeSystem));
});
SortedMap<String, Type> fieldMap = fields.build();
System.out.println("fieldMap = " + fieldMap);
return typeSystem.recordType(fieldMap);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it!
fieldMap = {todos={completed:bool, name:string} list}
fieldMap = {todos={completed:bool, name:string} list, users={age:int, name:string} list}
Out: val it = <relation> : {completed:bool, name:string} list
Updated with test, ready for review |
Thanks for the PR. In https://github.com/julianhyde/morel/tree/147-recursive-sub-schema I've fixed the formatting issues that were causing checkstyle to fail and added several other review comments. |
Merged. Thanks for the PR, @GavinRay97! |
Doesn't work currently, but also doesn't break existing code
I'm not sure how to create a new ML relation type and attach the results nested under that, but this is a decent scaffold of the logic (I hope)