Skip to content
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

Initial programmatic shell buildout #146

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

GavinRay97
Copy link
Contributor

@GavinRay97 GavinRay97 commented May 14, 2022

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)

dependabot bot and others added 30 commits February 28, 2022 20:16
Bumps [javacc](https://github.com/javacc/javacc) from 7.0.5 to 7.0.10.
- [Release notes](https://github.com/javacc/javacc/releases)
- [Changelog](https://github.com/javacc/javacc/blob/master/docs/release-notes.md)
- [Commits](javacc/javacc@7.0.5...javacc-7.0.10)

---
updated-dependencies:
- dependency-name: net.java.dev.javacc:javacc
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps `junit-jupiter.version` from 5.7.2 to 5.8.2.

Updates `junit-jupiter-api` from 5.7.2 to 5.8.2
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit5@r5.7.2...r5.8.2)

Updates `junit-jupiter-engine` from 5.7.2 to 5.8.2
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit5@r5.7.2...r5.8.2)

Updates `junit-jupiter-params` from 5.7.2 to 5.8.2
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit5@r5.7.2...r5.8.2)

---
updated-dependencies:
- dependency-name: org.junit.jupiter:junit-jupiter-api
  dependency-type: direct:development
  update-type: version-update:semver-minor
- dependency-name: org.junit.jupiter:junit-jupiter-engine
  dependency-type: direct:development
  update-type: version-update:semver-minor
- dependency-name: org.junit.jupiter:junit-jupiter-params
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [slf4j-api](https://github.com/qos-ch/slf4j) from 1.7.25 to 1.7.36.
- [Release notes](https://github.com/qos-ch/slf4j/releases)
- [Commits](qos-ch/slf4j@v_1.7.25...v_1.7.36)

---
updated-dependencies:
- dependency-name: org.slf4j:slf4j-api
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps jsr305 from 1.3.9 to 3.0.2.

---
updated-dependencies:
- dependency-name: com.google.code.findbugs:jsr305
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-surefire-plugin](https://github.com/apache/maven-surefire) from 3.0.0-M3 to 3.0.0-M5.
- [Release notes](https://github.com/apache/maven-surefire/releases)
- [Commits](apache/maven-surefire@surefire-3.0.0-M3...surefire-3.0.0-M5)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-surefire-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-site-plugin](https://github.com/apache/maven-site-plugin) from 3.7.1 to 3.11.0.
- [Release notes](https://github.com/apache/maven-site-plugin/releases)
- [Commits](apache/maven-site-plugin@maven-site-plugin-3.7.1...maven-site-plugin-3.11.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-site-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [jline](https://github.com/jline/jline3) from 3.16.0 to 3.21.0.
- [Release notes](https://github.com/jline/jline3/releases)
- [Changelog](https://github.com/jline/jline3/blob/master/changelog.md)
- [Commits](jline/jline3@jline-parent-3.16.0...jline-parent-3.21.0)

---
updated-dependencies:
- dependency-name: org.jline:jline
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [guava](https://github.com/google/guava) from 21.0 to 23.0.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](google/guava@v21.0...v23.0)

---
updated-dependencies:
- dependency-name: com.google.guava:guava
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Add 'order' clauses in tests where necessary to achieve
deterministic results; the order of iteration over
collections seems to have changed in recent versions of
Guava.
Bumps [javacc-maven-plugin](https://github.com/javacc/javacc-maven-plugin) from 3.0.0 to 3.0.3.
- [Release notes](https://github.com/javacc/javacc-maven-plugin/releases)
- [Commits](javacc/javacc-maven-plugin@javacc-maven-plugin-3.0.0...javacc-maven-plugin-3.0.3)

---
updated-dependencies:
- dependency-name: org.javacc.plugin:javacc-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.0.1 to 3.3.2.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.0.1...maven-javadoc-plugin-3.3.2)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-enforcer-plugin](https://github.com/apache/maven-enforcer) from 3.0.0-M1 to 3.0.0.
- [Release notes](https://github.com/apache/maven-enforcer/releases)
- [Commits](apache/maven-enforcer@enforcer-3.0.0-M1...enforcer-3.0.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-enforcer-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-project-info-reports-plugin](https://github.com/apache/maven-project-info-reports-plugin) from 2.9 to 3.2.2.
- [Release notes](https://github.com/apache/maven-project-info-reports-plugin/releases)
- [Commits](apache/maven-project-info-reports-plugin@maven-project-info-reports-plugin-2.9...maven-project-info-reports-plugin-3.2.2)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-project-info-reports-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
…, scott-data-hsqldb from 0.1 to 0.2

The foodmart-data-hsqldb and scott-data-hsqldb are necessary
because their previous versions used HSQLDB 1.8 file format,
which could not be read by HSQLDB higher than 2.5.1 or higher.
If you are running Java 11 or higher, you can use HSQLDB 2.6.1.
Make it easier to write tests that check for positions in
error messages, and write a unit test for Pos.

Fixes hydromatic#118
Implementation is brute force. If there are N variables, the
algorithm will try each combination of assigments of {false,
true} to variables, and stop when it finds one that works.

We can improve the implementation later, if necessary. For
example, if there is a group of variables where exactly one
is always true, then we can eliminate one of the variables.
(If exactly one of A, B and C is true, replace A with "not
(B or C)".)

For the environment, use a boolean[] rather than a
Map<Variable, Boolean>, exploiting the fact that variables have
contiguous integer ids.
… matches

Allow the shell to print warnings (even multiple warnings).

Add property 'matchCoverageEnabled', to control whether
coverage is analyzed. If it is false, an expression will
never be rejected for being redundant.

Add $bool and $list internal data types. This allows true,
false, NIL and CONS to be analyzed in matches the same way as
other constructors. Unlike values of other types, we know that
constructors are a finite set. So we can deduce that

  fun f true = ...
    | f false = ...

is exhaustive. Because $bool and $list are internal, they
can't be used in programs and don't appear in metadata.

Fixes hydromatic#55
Bumps [calcite-core](https://github.com/apache/calcite) from 1.29.0 to 1.30.0.
- [Release notes](https://github.com/apache/calcite/releases)
- [Commits](apache/calcite@calcite-1.29.0...calcite-1.30.0)

---
updated-dependencies:
- dependency-name: org.apache.calcite:calcite-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…0.0 to 3.1.2

Bumps [checkstyle](https://github.com/checkstyle/checkstyle) from 7.8.2 to 10.0.
- [Release notes](https://github.com/checkstyle/checkstyle/releases)
- [Commits](checkstyle/checkstyle@checkstyle-7.8.2...checkstyle-10.0)

Bumps [maven-checkstyle-plugin](https://github.com/apache/maven-checkstyle-plugin) from 3.0.0 to 3.1.2.
- [Release notes](https://github.com/apache/maven-checkstyle-plugin/releases)
- [Commits](apache/maven-checkstyle-plugin@maven-checkstyle-plugin-3.0.0...maven-checkstyle-plugin-3.1.2)

We now support and test checkstyle 9.3 and 10, but we use
checkstyle 9.3 by default because checkstyle 10.0 requires
JDK 11 or higher.

Update a few Checkstyle rules, and tighten the line length a
bit. Only check line length of Java files. Fix a few violations.

Stop using the net.hydromatic.toolbox.checkstyle Checkstyle
plugin. It apparently uses an obsolete Checkstyle SPI.
Maybe we'll re-enable it one day.

In POM, make plugin versions explicit.
Bumps [maven-source-plugin](https://github.com/apache/maven-source-plugin) from 2.2.1 to 3.2.1.
- [Release notes](https://github.com/apache/maven-source-plugin/releases)
- [Commits](apache/maven-source-plugin@maven-source-plugin-2.2.1...maven-source-plugin-3.2.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-source-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-compiler-plugin](https://github.com/apache/maven-compiler-plugin) from 2.3.2 to 3.10.1.
- [Release notes](https://github.com/apache/maven-compiler-plugin/releases)
- [Commits](apache/maven-compiler-plugin@maven-compiler-plugin-2.3.2...maven-compiler-plugin-3.10.1)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-compiler-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [build-helper-maven-plugin](https://github.com/mojohaus/build-helper-maven-plugin) from 1.9 to 3.3.0.
- [Release notes](https://github.com/mojohaus/build-helper-maven-plugin/releases)
- [Commits](mojohaus/build-helper-maven-plugin@build-helper-maven-plugin-1.9...build-helper-maven-plugin-3.3.0)

---
updated-dependencies:
- dependency-name: org.codehaus.mojo:build-helper-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps git-commit-id-plugin from 2.1.9 to 4.9.10.

---
updated-dependencies:
- dependency-name: pl.project13.maven:git-commit-id-plugin
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-surefire-plugin](https://github.com/apache/maven-surefire) from 3.0.0-M5 to 3.0.0-M6.
- [Release notes](https://github.com/apache/maven-surefire/releases)
- [Commits](apache/maven-surefire@surefire-3.0.0-M5...surefire-3.0.0-M6)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-surefire-plugin
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [javacc](https://github.com/javacc/javacc) from 7.0.10 to 7.0.11.
- [Release notes](https://github.com/javacc/javacc/releases)
- [Changelog](https://github.com/javacc/javacc/blob/master/docs/release-notes.md)
- [Commits](javacc/javacc@javacc-7.0.10...javacc-7.0.11)

---
updated-dependencies:
- dependency-name: net.java.dev.javacc:javacc
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [maven-site-plugin](https://github.com/apache/maven-site-plugin) from 3.11.0 to 3.12.0.
- [Release notes](https://github.com/apache/maven-site-plugin/releases)
- [Commits](apache/maven-site-plugin@maven-site-plugin-3.11.0...maven-site-plugin-3.12.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-site-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
dependabot bot and others added 5 commits April 20, 2022 21:44
Bumps [maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.3.2 to 3.4.0.
- [Release notes](https://github.com/apache/maven-javadoc-plugin/releases)
- [Commits](apache/maven-javadoc-plugin@maven-javadoc-plugin-3.3.2...maven-javadoc-plugin-3.4.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-javadoc-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [checkstyle](https://github.com/checkstyle/checkstyle) from 9.3 to 10.2.
- [Release notes](https://github.com/checkstyle/checkstyle/releases)
- [Commits](checkstyle/checkstyle@checkstyle-9.3...checkstyle-10.2)

---
updated-dependencies:
- dependency-name: com.puppycrawl.tools:checkstyle
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
If you are building under JDK 8, you must override Checkstyle
by adding `-Dcheckstyle.version=9.3` to the command line.

In GitHub actions, downgrade to setup-java from v2 to v1,
because apparently only v1 supports Java 18.
Bumps [maven-project-info-reports-plugin](https://github.com/apache/maven-project-info-reports-plugin) from 3.2.2 to 3.3.0.
- [Release notes](https://github.com/apache/maven-project-info-reports-plugin/releases)
- [Commits](apache/maven-project-info-reports-plugin@maven-project-info-reports-plugin-3.2.2...maven-project-info-reports-plugin-3.3.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-project-info-reports-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@GavinRay97 GavinRay97 marked this pull request as ready for review May 14, 2022 21:40
@GavinRay97 GavinRay97 changed the title WIP: Initial programmatic shell buildout Initial programmatic shell buildout May 14, 2022
@GavinRay97
Copy link
Contributor Author

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
Copy link
Collaborator

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 *)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants