search

ActianCorp / actian_tableau_connector

Tableau connector (aka taco) for Actian Avalanche, Vector, and Ingres
https://extensiongallery.tableau.com/products/936
Apache License 2.0
1 stars 4 forks source link

sync/compare code for ODBC and JDBC connectors #27

Closed clach04 closed 1 year ago

clach04 commented 1 year ago

From https://github.com/ActianCorp/actian_tableau_connector/pull/26 also related to https://github.com/ActianCorp/actian_tableau_connector/issues/24

ODBC xml file has less/different items in compared with the JDBC one

https://github.com/ActianCorp/actian_tableau_connector/blob/e7aae215602c84f28cbf8f14f33b0995e5bc1707/actian_odbc/manifest.xml#L56

https://github.com/ActianCorp/actian_tableau_connector/blob/e7aae215602c84f28cbf8f14f33b0995e5bc1707/actian_jdbc/manifest.xml#L58

diff -r actian_odbc actian_jdbc returns more diffs than I was expecting:

  1. Majority of diffs are real js diff - which ARE expected
  2. some (white space?) XML diffs with what look like the same values. Recommend we clobber these and make them match to remove chaff
  3. Changes / TODO notes that need follow up, possible with Tableau/SalesForce - i.e. TechDebt we knew about but hadn't pursued as ODBC had been the only viable option... until now NOTE See https://tableau.github.io/connector-plugin-sdk/docs/gallery-submission#step-1-build-and-test-your-connector since April 2022 "Only JDBC connectors are accepted."
  4. Changes that need explaining/resolving

Majority of diffs are real js diff

Note shown, basically all the js code :-) - not interesting for this issue.

some (white space?) XML diffs

diff -r actian_odbc\manifest.xml actian_jdbc\manifest.xml
...
27c29
<       <customization name="CAP_INDEX_TEMP_TABLES" value="yes"/>
---
>       <customization name="CAP_INDEX_TEMP_TABLES" value="yes"/>

.. and more. See comment https://github.com/ActianCorp/actian_tableau_connector/issues/27#issuecomment-1681286854

Changes / TODO notes that need follow up

NOTE on JDBC requirement since 2022

Code diffs of interest:

diff -r actian_odbc\manifest.xml actian_jdbc\manifest.xml
...
19,23c19,25
<       <!-- See https://tableau.github.io/connector-plugin-sdk/docs/capabilities.html for capabilities names and explanations -->
<       <customization name="CAP_ODBC_BIND_DETECT_ALIAS_CASE_FOLDING" value="yes"/>
<       <customization name="CAP_ODBC_METADATA_FORCE_NUM_PREC_RADIX_10" value="yes"/>
<       <customization name="CAP_ODBC_METADATA_FORCE_LENGTH_AS_PRECISION" value="yes"/>
<       <customization name="CAP_ODBC_SUPPRESS_INFO_SCHEMA_TABLES" value="yes"/>
---
>       <!--
>       look at odbc connector and review CAP_ODBC_* for corresponding jdbc entries
>       these are now documented as of mid Aug 2019 at
>       https://tableau.github.io/connector-plugin-sdk/docs/capabilities.html#jdbc
>       -->
>       <customization name="CAP_JDBC_BIND_DETECT_ALIAS_CASE_FOLDING" value="yes"/>
>

Changes that need explaining/resolving

diff -r actian_odbc\manifest.xml actian_jdbc\manifest.xml
...
56c58,59
<   <connection-dialog file='connection-dialog.tcd'/>
---
>   <connection-fields file='connectionFields.xml'/>
>   <connection-metadata file='connectionMetadata.xml'/>
clach04 commented 1 year ago

@hab6 can you do me a favor and help me figure out the TODO item in the report? *TODO link to "no longer accept ODBC tacos"

Can you point me at the website/page that says we no longer accept ODBC? Thanks!

hab6 commented 1 year ago

The Connector Tableau Exchange Submission Guide doc page has the following statement in section Step 1: Build and test your connector

Note: The Tableau Exchange no longer accepts submissions of ODBC connectors. Only JDBC connectors are accepted.

clach04 commented 1 year ago

Thanks @hab6, description updated along with some timeline info.

I recommend we make ODBC and JDBC code identical as possible so that we can diff them to make it easier to see the wheat from the chafe before tackling https://github.com/ActianCorp/actian_tableau_connector/issues/25 (unless you already have positive changes/progress already). I have a hunch the number 2 items may explain some issues, the number of entries is suspiciously close to the number differences in failures, it could be a coincidence but easy to make the code match and then focus on diffs to remove any lingering doubt.

clach04 commented 1 year ago

For white space diffs. see attached diff for:

diff -w -r actian_odbc actian_jdbc

looks "good" to me, so I think we can clobber the white space diffs.

ignore_whitespace.diff.txt

hab6 commented 1 year ago

@clach04 PR #28 - minor whitespace and formatting changes

hab6 commented 1 year ago

Referencing comment, item #3 - Code diffs of interest

diff -r actian_odbc\manifest.xml actian_jdbc\manifest.xml
...
19,23c19,25
<       <!-- See https://tableau.github.io/connector-plugin-sdk/docs/capabilities.html for capabilities names and explanations -->
<       <customization name="CAP_ODBC_BIND_DETECT_ALIAS_CASE_FOLDING" value="yes"/>
<       <customization name="CAP_ODBC_METADATA_FORCE_NUM_PREC_RADIX_10" value="yes"/>
<       <customization name="CAP_ODBC_METADATA_FORCE_LENGTH_AS_PRECISION" value="yes"/>
<       <customization name="CAP_ODBC_SUPPRESS_INFO_SCHEMA_TABLES" value="yes"/>
---
>       <!--
>       look at odbc connector and review CAP_ODBC_* for corresponding jdbc entries
>       these are now documented as of mid Aug 2019 at
>       https://tableau.github.io/connector-plugin-sdk/docs/capabilities.html#jdbc
>       -->
>       <customization name="CAP_JDBC_BIND_DETECT_ALIAS_CASE_FOLDING" value="yes"/>

Of the 4 ODBC capabilities listed in the diff, only one has a JDBC equivalent (see the Tableau Capabilities page for details). Summary of the listed diffs here:

ODBC Capability JDBC Capability
CAP_ODBC_BIND_DETECT_ALIAS_CASE_FOLDING CAP_JDBC_BIND_DETECT_ALIAS_CASE_FOLDING
CAP_ODBC_METADATA_FORCE_NUM_PREC_RADIX_10 No equivalent for JDBC
CAP_ODBC_METADATA_FORCE_LENGTH_AS_PRECISION No equivalent for JDBC
CAP_ODBC_SUPPRESS_INFO_SCHEMA_TABLES No equivalent for JDBC

Therefore, I think no action is required to sync up the <customizations> section between the ODBC and JDBC manifest.xml files.

clach04 commented 1 year ago

Closing as complete/done.