jnturton commented on PR #2551: URL: https://github.com/apache/drill/pull/2551#issuecomment-1129938286
> I have two comments: > > 1. We might want to change the log level to `warn` here instead of info. > > 2. What happens if we eat the error message? Does the schema still get defined somewhere? I didn't see that anywhere and it looks like it is causing an NPE in one of the unit tests. More unit tests would be nice as well. I'll change it to a warning. Already present in the plugin code (not in this PR) are an attempt to default the schema in the event that the JDBC driver doesn't supply it, and also logic for the case when both routes fail and leaving `defaultSchema = null`. I'll add a MSSQL testcontainer and new test suite since that's the only sane way I can see to unit test this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
