Allow sqlline customizations (supported options, product version, the startup quotes)

[arina-ielchiieva]

Add ability to define the options supported, the default startup options, the product version, and the
start up quotes to an external resource file that is optionally passed to SQLLine at startup.

All of that is hard-coded in Drill fork (https://github.com/mapr/sqlline/commits/1.1.9-drill-r7) but ideally that should be loaded from external file.
Example of custom product and start up quotes:

apache drill 1.15.0-SNAPSHOT
"got drill?"

[julianhyde]

I support this; however, properties may be an alternative way of customizing the startup message. Properties can be specified using the !set command (not much use for startup message!), stored in a config file, or passed as command-line arguments. I suggest you investigate properties as an additional/alternative way of customizing.

[arina-ielchiieva]

@julianhyde I have submitted the PR allowing to customize start up message using --appInfo=class_name.

Though I have one question: currently Drill execute command is looking like CMD="$JAVA $SHELL_OPTS -cp $CP sqlline.SqlLine -d org.apache.drill.jdbc.Driver --maxWidth=10000 --isolation=TRANSACTION_NONE --incremental=false --appInfo=org.apache.drill.exec.client.DrillSqllineAppInfo", so I was looking for a way to pass options in a file. Tried CMD="$JAVA $SHELL_OPTS -cp $CP sqlline.SqlLine -d org.apache.drill.jdbc.Driver sqlline.properties". sqlline.properties is in classpath and contains the following:

maxWidth=10000
isolation=TRANSACTION_NONE
incremental=false
appInfo=org.apache.drill.exec.client.DrillSqllineAppInfo

though SqlLine complains it cannot find the file. Using -properties / --properties did not help as well, but according to the description I assume this command is for connection purposes.

Does sqlline suport passing options in properties files on start up from the file in classpath? Or I am doing something wrong.

[julianhyde]

Reviewing PR #118.

I like the idea of AppInfo... should we take it further? Should the application class also control the set of commands that are loaded on startup? What else? (I'd rather have one extension point than several.)

I think that there should be a public base class, and we should recommend that people extend that. So that they won't be broken if we extend the interface in future.

Thanks for adding tests & doc.

[arina-ielchiieva]

Sounds like a nice idea. I think AppInfo (might be renamed) can allow to customize (i.e. add, remove) besides the info message:

• output formats
• isolation levels
• commands
• known drivers
• connection url examples

[snuyanzin]

I think it could also be used to set default values for variables so that !set <key> default will use the values from the class extension if it is present and from core sqlline if not. + There could be something like a global !reset to reset all variables to values based on the customized class

[arina-ielchiieva]

@julianhyde updated the PR: new class ApplicationConfig contains information about: info message, output formats, commands, drivers and their examples. User may override init methods to modify original values. Class name can be passed to Sqlline using -ac or !appconfig. Please take a look.

[julianhyde]

I've merged your PR into https://github.com/julianhyde/sqlline/tree/scratch and am now testing. @arina-ielchiieva and @snuyanzin can you review the changes I made.

[arina-ielchiieva]

@julianhyde this was unexpected :) Thank you for making the changes!
Tested against scratch branch and have two comments:

1. OutputFormat and AbstractOutputFormat should be made public so user can create own formats and be able to override public Map<String, OutputFormat> getOutputFormats(SqlLine sqlLine) method.
2. initDrivers and getConnectionUrlExamples return immutable collections while getOutputFormats and getCommandHandlers don't. When overriding these methods, I would kind of expect similar behavior for all methods: all of them to return mutable collections.

[snuyanzin]

Thank you for merging

1. Pulled scratch branch, compiled as mvn clean install - OK
2. Bumped dependency in Apache Calcite to sqlline-1.5.0-SNAPSHOT and compiled - OK
3. Created custom command handler in calcite-csv-example, compiled and added via commandhandler and -ch, checked simple new commands - OK
4. Created custom application configuration in calcite-csv-example - NOT OK as NOT compiled
   as mentioned by @arina-ielchiieva to make it compilable interface sqlline.OutputFormat has to be public as it is mentioned in the method sqlline.Application#getOutputFormats
   • After making sqlline.OutputFormat public everything compiled. Added custom application configuration via -ac and !appconfig - OK
5. Minor comment about manual.txt I see there were added line about !appconfig however there is not commandhandler and lots of others like !call, !nickname, !nativesql, !typeinfo.
   Should be actual or not?

[julianhyde]

4. Created custom application configuration in calcite-csv-example - NOT OK as NOT compiled
   as mentioned by @arina-ielchiieva to make it compilable interface sqlline.OutputFormat has to be public as it is mentioned in the method sqlline.Application#getOutputFormats

I like how in 23149f1 you created a test HelloWorldCommandHandler in a different package. We could use something similar here.

5. Minor comment about manual.txt I see there were added line about !appconfig however there is not commandhandler and lots of others like !call, !nickname, !nativesql, !typeinfo. Should be actual or not?

Yes, all commands should be there. Can you log an issue for that?

[julianhyde]

@arina-ielchiieva,

OutputFormat and AbstractOutputFormat should be made public so user can create own formats and be able to override public Map<String, OutputFormat> getOutputFormats(SqlLine sqlLine) method.

I agree. Can you please fix that, and also move CustomApplication to the sqlline.commandhandler package to keep us honest. (Yeah, the package name isn't great. But it will do.)

2. initDrivers and getConnectionUrlExamples return immutable collections while getOutputFormats and getCommandHandlers don't. When overriding these methods, I would kind of expect similar behavior for all methods: all of them to return mutable collections.

I agree we need consistency. I would slightly prefer unmodifiable/immutable collections. Even though that means that the overriding method has to make a copy.

Any chance you can make these changes in the next few days? This is the last issue to fix before 1.5 Rc0.

[snuyanzin]

Yes, all commands should be there. Can you log an issue for that?

sure, done #127

[julianhyde]

Dev branch is now https://github.com/julianhyde/sqlline/tree/106-application.

[arina-ielchiieva]

Opened PR against 106-application branch - #130.

[arina-ielchiieva]

Fixed in 5d4c0a5.