ODBC: Adding BASIC & AWS_SIGV4 auth in M Connector #610
Conversation
- add docs
- add unit test - add errors - enable direct query
…e/odbc/pbi # Conflicts: # sql-odbc/.gitignore # sql-odbc/src/PowerBIConnector/OdfeSqlOdbcPBIConnector.mproj # sql-odbc/src/PowerBIConnector/OdfeSqlOdbcPBIConnector.pq # sql-odbc/src/PowerBIConnector/OdfeSqlOdbcPBIConnector.query.pq # sql-odbc/src/PowerBIConnector/bin/Release/OdfeSqlOdbcPBIConnector.mez
|
|
||
| // Add support for `LIMIT` and `OFFSET` clauses (rather than `TOP`) | ||
| AstVisitor = [ | ||
| // format is "LIMIT [<skip>,]<take>" - ex. LIMIT 2,10 or LIMIT 10 |
There was a problem hiding this comment.
Can you verify whether ODFE SQL supports this [skip,]take syntax? If it doesn't, then we may want to change this to LIMIT <take> OFFSET <skip>
There was a problem hiding this comment.
[skip,]take syntax is supported
| GroupByCapabilities = ODBC[SQL_GB][SQL_GB_COLLATE] | ||
| ]), | ||
|
|
||
| SQLGetInfo = Diagnostics.LogValue("SQLGetInfo_Options", [ |
There was a problem hiding this comment.
These are values that we can change at the driver level. Unless those changes break other applications, lets set Capabilities and GetInfo values there instead
Can you remove these lines (SQLCapabilities & SQLGetInfo) and verify if we get the same connection behaviour?
There was a problem hiding this comment.
Removed SQLCapabilities & SQLGetInfo since all required values are defined at the driver level
| ] | ||
| ], | ||
|
|
||
| OdbcDatasource = Odbc.DataSource(ConnectionString & Server & CredentialConnectionString, [ |
There was a problem hiding this comment.
Shouldn't this concatenate EncryptedConnectionString too?
| ], | ||
|
|
||
| // Enable Encryption | ||
| SupportsEncryption = true, |
There was a problem hiding this comment.
Note that if you set this to true, PBI will always try encrypted connections. You need to add an error handler that tells PBI to try an unencrypted connection if it gets an SSL error. See the ImpalaODBC example on the PowerQuery SDK github.
There was a problem hiding this comment.
Yes, will include that in separate PR since some changes are required in driver code to return proper error code for SSL.
Issue #595
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.