You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Calcite 1.39.0 has a change to better support large numbers of schemas. see https://issues.apache.org/jira/browse/CALCITE-6728; this has resulted in a change to the internals for the org.apache.calcite.jdbc.SimpleCalicateSchema; this class is extended by LookupCalciteSchema in the Isthmus codebase. As this subclass has removed and altered its protected methods, fails Ishtmus fails to compile (though if you run without recompiling, errors relating to missing tables abound)
(aside: there is an argument that the protected methods of non-final classes should be considered an API and not changed; however, from a best-practice point of view, irrespective of any changes, it would be better not to adopt the pattern LookupCalciteSchema is following. i.e. having to be in the same package as a library you are dependent on and subclassing.)
This LookupCalciteSchema class is used extensively by the conversions from Substrait to Calcite & SQL.
I can resolve this by changing toSchema(Rel rel) method in the SubstraitToCalcite codebase - something similar to this
protected CalciteSchema toSchema(Rel rel) {
Map<List<String>, NamedStruct> tableMap = NamedStructGatherer.gatherTables(rel);
Map<String, Table> tables = tableMap.entrySet().stream().map(entry -> {
var id = entry.getKey();
var table = entry.getValue();
var value = new SqlConverterBase.DefinedTable(
id.get(id.size() - 1),
typeFactory,
typeConverter.toCalcite(typeFactory, table.struct(), table.names()));
return Map.entry(id.get(id.size() - 1), value);
}).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
var schema = new AbstractSchema(){
@Override
protected Map<String, Table> getTableMap() {
return tables;
}
};
return CalciteSchema.createRootSchema(false, false, "", schema);
}
This gives a 99% pass rate without the 4 tests failing in LookupTableTest - however as this test is specifically relying on the deferred lookup of the table names; which (ironically) is what the new feature in calcite is intended to support. This knocks a couple more tests off;
What's left are two namespace tests... where the namespace tablename org.example.FOO say is coming out in calcite as just being FOO....
UPDATE: solution has been found!
Tidying up the solution ahead of a PR.
The text was updated successfully, but these errors were encountered:
Uh oh!
There was an error while loading. Please reload this page.
Calcite 1.39.0 has a change to better support large numbers of schemas. see https://issues.apache.org/jira/browse/CALCITE-6728; this has resulted in a change to the internals for the
org.apache.calcite.jdbc.SimpleCalicateSchema
; this class is extended byLookupCalciteSchema
in the Isthmus codebase. As this subclass has removed and altered its protected methods, fails Ishtmus fails to compile (though if you run without recompiling, errors relating to missing tables abound)(aside: there is an argument that the protected methods of non-final classes should be considered an API and not changed; however, from a best-practice point of view, irrespective of any changes, it would be better not to adopt the pattern
LookupCalciteSchema
is following. i.e. having to be in the same package as a library you are dependent on and subclassing.)This
LookupCalciteSchema
class is used extensively by the conversions from Substrait to Calcite & SQL.I can resolve this by changing
toSchema(Rel rel)
method in theSubstraitToCalcite
codebase - something similar to thisThis gives a 99% pass rate without the 4 tests failing in
LookupTableTest
- however as this test is specifically relying on the deferred lookup of the table names; which (ironically) is what the new feature in calcite is intended to support. This knocks a couple more tests off;What's left are two namespace tests... where the namespace tablename
org.example.FOO
say is coming out in calcite as just beingFOO
....UPDATE: solution has been found!
Tidying up the solution ahead of a PR.
The text was updated successfully, but these errors were encountered: