Make methods and classes required for extensions public

[mihaibudiu]

Fixes #24
I hope I got all the required ones.
This is required to complete https://issues.apache.org/jira/browse/CALCITE-5615

[julianhyde]

Public classes and methods should have a javadoc description.

There are various inner classes called Factory. I think these classes should be private; the enclosing classes should have a public static final field called FACTORY, instead of the current INSTANCE field inside each Factory class.

Let's not make release 0.2 until we have CALCITE-5615 working. It should be possible to run Calcite against a sql-logic-test:0.2-SNAPSHOT deployed to a local Maven or Gradle repository.

[zabetak]

FYI: To use maven local in Calcite build you have to use the -PenableMavenLocal (https://github.com/apache/calcite/blob/a833bd626c5e781ccc27bd6fab1c636a3b944907/build.gradle.kts#L512)

[mihaibudiu]

I have moved the factory instances to their parents, but making the factory classes private does not allow me to call 'register'. I will submit a PR soon with multiple commits, each addressing some of these suggestions.

[mihaibudiu]

In fact, I will add commits to this PR.

[mihaibudiu]

I have pushed several commits which attempt to fix all the problems raised.
I have checked that adding this JAR to calcite enables the CalciteExecutor to be implemented (using implementation(files()) in build.gradle.kts).
I will update apache/calcite#3145 once this is published. The resulting code will be tiny, with just 2 files.
The Calcite built system is complicated, so I hope I haven't overlooked anything.

[mihaibudiu]

Fixes #26

[julianhyde]

It doesn't matter very much whether it's one commit or multiple. But I don't yet see a clean abstraction to deal with extensions. Making int fields public, and methods without javadoc, are symptoms of this.

Also, the lifecycle of Factory seems strange. I'm not sure who would use a Factory, rather than calling ExecutionOptions.registerExecutor directly? Also, I don't see why a class that implements Factory being private prevents you from calling the register method.

I agree that an abstract base class for JdbcExecutor would be useful. Or perhaps move the implementation of various methods from CalciteExecutor to JdbcExecutor, and let derived classes override if they wish.

[mihaibudiu]

If I make (one of) the Factory private I get this error:

[ERROR] /home/mbudiu/git/sql-logic-test/src/main/java/net/hydromatic/sqllogictest/Main.java:[54,27] net.hydromatic.sqllogictest.executors.HsqldbExecutor.Factory.register(net.hydromatic.sqllogictest.ExecutionOptions) is defined in an inaccessible class or interface

I generally make final fields public, but I have no problem reorganizing if you prefer.
I have added JavaDoc to all public methods I could find. But many weren't related to extensibility.
Let me think a bit about whether I can simplify the factories, perhaps they are not necessary.

[mihaibudiu]

Yes, the Factory can be removed if I make the 'register' method a static method of the Executor.
I created the ExecutorFactory because you can't have static methods in interfaces, but I can remove that.

[mihaibudiu]

I have removed the Factory classes.
Both HsqldbExecutor and PostgresExecutor are built on top of the JdbcExecutor.
The 'register' calls are the means to add new executors. That's how extensibility works.
I am happy to rework the design if you have a different proposal.

[mihaibudiu]

One thing that is not extensible is (what is now in) the Main class. In the CalciteExecutor I just defined a new one.
Perhaps that can be improved.

[julianhyde]

This change is not necessary if you rebase onto latest main.

[mihaibudiu]

I have rebased on main

[mihaibudiu]

Please note that I have changed my github user id. I don't plan to do this again very soon, but I am leaving VMware Research next week and I didn't want my id to be tied to my employer. (That was a mistake anyway.)