hydromatic / morel

Standard ML interpreter, with relational extensions, implemented in Java
Apache License 2.0
291 stars 15 forks source link

Initial programmatic shell buildout #146

Open GavinRay97 opened 2 years ago

GavinRay97 commented 2 years ago

This is currently failing in the test, and I've tried to debug but unsure why (attempted to mirror the existing Shell code):

EDIT: Removing static from the variable fixed it.

java.lang.NullPointerException: metadataHandlerProvider

    at java.base/java.util.Objects.requireNonNull(Objects.java:233)
    at org.apache.calcite.rel.metadata.RelMetadataQueryBase.getMetadataHandlerProvider(RelMetadataQueryBase.java:122)
    at org.apache.calcite.rel.metadata.RelMetadataQueryBase.revise(RelMetadataQueryBase.java:118)
    at org.apache.calcite.rel.metadata.RelMetadataQuery.collations(RelMetadataQuery.java:604)
    at org.apache.calcite.rel.metadata.RelMdCollation.project(RelMdCollation.java:291)
    at org.apache.calcite.rel.logical.LogicalProject.lambda$create$0(LogicalProject.java:125)
    at org.apache.calcite.plan.RelTraitSet.replaceIfs(RelTraitSet.java:244)
    at org.apache.calcite.rel.logical.LogicalProject.create(LogicalProject.java:124)
    at org.apache.calcite.rel.logical.LogicalProject.create(LogicalProject.java:114)
    at org.apache.calcite.rel.core.RelFactories$ProjectFactoryImpl.createProject(RelFactories.java:178)
    at org.apache.calcite.tools.RelBuilder.project_(RelBuilder.java:2025)
    at org.apache.calcite.tools.RelBuilder.project(RelBuilder.java:1797)
    at net.hydromatic.morel.foreign.CalciteForeignValue.lambda$value$4(CalciteForeignValue.java:100)
    at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:422)
    at com.google.common.collect.RegularImmutableSortedSet.forEach(RegularImmutableSortedSet.java:89)
    at net.hydromatic.morel.foreign.CalciteForeignValue.value(CalciteForeignValue.java:89)
    at net.hydromatic.morel.compile.Environments.lambda$foreignBindings$2(Environments.java:104)
    at com.google.common.collect.RegularImmutableMap.forEach(RegularImmutableMap.java:292)
    at net.hydromatic.morel.compile.Environments.foreignBindings(Environments.java:101)
    at net.hydromatic.morel.compile.Environments.env(Environments.java:95)
    at net.hydromatic.morel.compile.Environments.env(Environments.java:63)
    at net.hydromatic.morel.ProgrammaticShell.makeEnv(ProgrammaticShell.java:80)
    at net.hydromatic.morel.ProgrammaticShell.<init>(ProgrammaticShell.java:54)
    at net.hydromatic.morel.ProgrammaticShellTest.run(ProgrammaticShellTest.java:19)
GavinRay97 commented 2 years ago

A note about the Javadoc on the class that mentions caching statements to results:

I tried to implement this but it turned out to be a bit trickier than I thought Had started writing a class, EvaluationContext, which holds an Environment + Map<String, ForeignValue> + AstNode

So I thought that a Cache<EvaluationContext, CompiledStatement> could be implemented. But the hashing doesn't seem to work, I always get cache misses =/

Implementing .equals() and .hashCode() on AstNode and MapEnvironment don't seem to change this 🤔

public class ProgrammaticShell implements Session.Shell {
    // ...
    private boolean enableCache = true;
    private Cache<EvaluationContext, CompiledStatement> compiledStatementCache =
            CacheBuilder.newBuilder().recordStats().build();
    // ....

      private void command(AstNode statement, Consumer<String> outLines) {
          try {
              final Map<String, Binding> outBindings = new LinkedHashMap<>();
              final Environment env = env0.bindAll(outBindings.values());

              final List<Binding> bindings = new ArrayList<>();
              CompiledStatement compiled;

              // If cache is enabled
              if (enableCache) {
                  EvaluationContext context = new EvaluationContext(env0, foreignValueMap, statement);
                  CompiledStatement compiledStatement = compiledStatementCache.getIfPresent(context);
                  // If not in cache, compile and put in cache
                  if (compiledStatement == null) {
                      compiled = Compiles.prepareStatement(typeSystem, session, env,
                                      statement, null, e -> appendToOutput(e, outLines));
                      compiledStatementCache.put(context, compiled);
                  } else {
                      // If in cache, use cached compiled statement
                      compiled = compiledStatement;
                  }
              } else {
                  // If cache is disabled, compile and don't put in cache
                  compiled = Compiles.prepareStatement(typeSystem, session, env,
                                  statement, null, e -> appendToOutput(e, outLines));
              }

              compiled.eval(session, env, outLines, bindings::add);
              bindings.forEach(b -> outBindings.put(b.id.name, b));
          } catch (Codes.MorelRuntimeException e) {
              appendToOutput(e, outLines);
          }
      }
}
final class EvaluationContext {
    private final Session session;
    private final Environment env;
    private final Map<String, ForeignValue> foreignValueMap;

    EvaluationContext(
            Session session,
            Environment env,
            Map<String, ForeignValue> foreignValueMap) {
        this.session = session;
        this.env = env;
        this.foreignValueMap = foreignValueMap;
    }

    public Session session() {
        return session;
    }

    public Environment env() {
        return env;
    }

    public Map<String, ForeignValue> foreignValueMap() {
        return foreignValueMap;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == this) return true;
        if (obj == null || obj.getClass() != this.getClass()) return false;
        var that = (EvaluationContext) obj;
        return Objects.equals(this.session, that.session) &&
               Objects.equals(this.env, that.env) &&
               Objects.equals(this.foreignValueMap, that.foreignValueMap);
    }

    @Override
    public int hashCode() {
        return Objects.hash(session, env, foreignValueMap);
    }

    @Override
    public String toString() {
        return "EvaluationContext[" +
               "session=" + session + ", " +
               "env=" + env + ", " +
               "foreignValueMap=" + foreignValueMap + ']';
    }
}
julianhyde commented 2 years ago

Your EvaluationContext looks fairly similar to the bindingMap field in Main.Shell.

I wouldn't use a Cache for these purposes, because we don't want bindings to disappear, there's no reliable way to re-create a value that has been dropped from the cache (if Morel ever becomes even a little bit impure).

In ML and Morel bindings in the shell become inaccessible if another binding has been created with the same name. Then the garbage collector will take its course. The values, of course, may still be referenced via closures:

val x = 1;
val addX = fn y => y + x;
addX 2; (* returns 3 *)
val x = 5; (* makes the previous binding of x invisible *)
addX 2; (* still returns 3 *)