apache / hop

Hop Orchestration Platform
https://hop.apache.org/
Apache License 2.0
898 stars 328 forks source link

[Feature Request]: Database plugin query comment injection #3261

Open tpickle-amazon opened 9 months ago

tpickle-amazon commented 9 months ago

What would you like to happen?

Hello, opening this feature request as a follow-up to this thread: https://chat.project-hop.org/hop/pl/sx5ye8z5ntf9xyu9eab1ibgqwy. My team is developing our own Hop database plugin (i.e. extending BaseDatabaseMeta) and have a use case in which we would like to include a comment in every SQL query sent using our custom database plugin. Ideally, this functionality would be specified in the IDatabase interface and available to implement/override in our plugin implementation. This would be similar to the existing: getConnectSql() method of the IDatabase interface. So something like:

interface IDatabase {
    String getSqlComment();
}

Then this getSqlComment() method could be called by the Database class when executing queries, similar to the Database class calling getConnectSql() when connecting. Happy to answer any questions, thanks for the consideration!

Issue Priority

Priority: 3

Issue Component

Component: Database

mattcasters commented 8 months ago

Sounds like a cool idea. Perhaps we could make this configurable in the GUI as well so that it works for all databases? After all, the hard work is the injection of the comment before the SQL in Database:openQuery, not the interface change. We have a command to execute after connecting. We could have a section below called Inject the following comment in database queries:. Is this needed for insert/update/delete/lookup/... as well?

tpickle-amazon commented 7 months ago

Is this needed for insert/update/delete/lookup/... as well?

we are mostly concerned with read queries, so insert/update/delete are probably not needed for now, but lookup would be ideal.

tpickle-amazon commented 7 months ago

One potential approach might be to use the ClientInfo functionality of the JDBC: https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#setClientInfo-java.util.Properties. Based on these docs(see clientInfoProvider) it seems like JDBC Driver implementations (such as Mysql) can support ClientInfo in different ways. Based on the above doc, the default clientInfoProvider for MySql is com.mysql.cj.jdbc.CommentClientInfoProvider which injects the ClientInfo properties as comments into the query. So wondering if it would better align with the JDBC interface/library to use ClientInfo properties instead of getSqlComment().

Rather than adding:

interface IDatabase {
    String getSqlComment();
}

could have:

interface IDatabase {
    Properties getClientInfoProperties();
}

then during the connect() routine of the main Database.java have something like:

if databasePlugin.getClientInfo() != null {
    connection.setClientInfo(databasePlugin.getClientInfoProperties());
}

and let the JDBC driver implementation handle the specifics (in our case this would be Mysql + CommentClientInfoProvider).