Feature/pmi 69 invert dialect dependencies #31

redcatbear · 2018-09-07T10:29:59Z

@andrehacker please review the changes.

Overview:

Changed dialects registry to have fewer coupling
Externalized list of dialects to property file
Improved test coverage for dialects registry
Improved logging
Introduced logging parameter in CREATE VIRTUAL SCHEMA command
Improve documentation
Moved public static NAME in dialects to public static method (cleaner than exposing a constant - even if not by much)
Some refactoring to remove warnings
Removed dead code where I found it
Improved Travis script for integration testing

…on test script.

andrehacker

Great to see that you cared for the code and refactored it by finals, new logging, etc. Thank you! Just have some minor comments - will be glad to see it merged!

andrehacker · 2018-10-01T17:28:11Z

jdbc-adapter/doc/deploy-adapter.md

 * Advanced edition (which includes the ability to execute adapter scripts), or Free Small Business Edition
-* EXASOL must be able to connect to the host and port specified in the JDBC connection string. In case of problems you can use a [UDF to test the connectivity](https://www.exasol.com/support/browse/SOL-307).
-* If the JDBC driver requires Kerberos authentication (e.g. for Hive or Impala), the EXASOL database will authenticate using a keytab file. Each EXASOL node needs access to port 88 of the the Kerberos KDC (key distribution center).
+* Exasol must be able to connect to the host and port specified in the JDBC connection string. In case of problems you can use a [UDF to test the connectivity](HTTPS://www.exasol.com/support/browse/SOL-307).


cosmetic: any reason for HTTPS capital case here?

Search-and-replace gone wrong.
Fixed.

andrehacker · 2018-10-01T17:52:24Z

jdbc-adapter/doc/develop-dialect.md

+
+* Make sure the data in the source database is not confidential (demo data only)
+* Don't reuse credentials
+* Don't check in credentials


good points!

Speaking from experience a lot of junior developers and even seniors in a hurry need a friendly reminder now and then. :-)

andrehacker · 2018-10-01T17:56:00Z

jdbc-adapter/doc/develop-dialect.md

+* Test data loaded into source database
+* [BucketFS HTTP port listening and reachable](https://www.exasol.com/support/browse/SOL-503?src=confmacro)
+
+  ![BucketFS on port 2580](images/Screenshot_BucketFS_default_service.png)


The red highlight can be misunderstood in the way that it has to be port 2580. Maybe extending the previous sentence might clarify it:

BucketFS HTTP port (e.g. 2580) listening and reachable

andrehacker · 2018-10-01T17:57:57Z

jdbc-adapter/doc/develop-dialect.md

-## How To Start Integration Tests
-We assume that you have a running EXASOL and data source database with all required test tables.
+1. Create a dedicated user in the source database that has the necessary access privileges 
+2. Create credentials for the user with which 


sentence incomplete...

andrehacker · 2018-10-01T18:09:43Z

...virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/AbstractSqlDialect.java

+                    .getPrecisionOrSize()) ? DataType.ExaCharset.ASCII : DataType.ExaCharset.UTF8;
+            if (jdbcTypeDescription.getPrecisionOrSize() <= DataType.maxExasolCharSize) {
+                colType = DataType.createChar(jdbcTypeDescription.getPrecisionOrSize(), charset);
+            } else {


Hard to read all these diffs in the switch, but I guess it is just formatting.

Once we really refactor this kind methods in earnest of that size need to be split up.
Suggestion: let's setup Sonar and fix the Sonar findings in a separate branch. Methods like this one here will most certainly raise an alarm.

@OverRide

* Sybase Virtual Schema (#32) * Use SqlServer dialect as starting point * Add sybase dialect and override for ORDER BY * Setup Sybase integration tests: includes order by tests * Add WHERE test * Date/time types: handle conversion and add tests * Add sybase integer datatype tests * Add a whole bunch of data type tests * Add binary, varbinary, image and bit tests * Add Sybase to the supported dialects * Pmi 73 take over sybase from hendrik (#33) * PMI-16: Set fixed database version for CI build. Cleaned up integration test script. * PMI-16: Fixed quoting. * PMI-16: Corrected docker image version. * PMI-76: Cherry-picked integration test script improvements. * Pmi 74 improve travis ci build (#35) * PMI-74: Used "mktemp". Improved configurability and readability. * PMI-74: Parameterized more hard-coded paths. * PMI-74: Change temp dir to rooted temp dir. * PMI-74: Improved readability by splitting file into functions. * PIM-74: Travis badge now points to default branch. * Update README.md * Added DBeaver files to ".gitignore" * Feature/pmi 69 invert dialect dependencies (#31) * refactor(PMI-69): Turned hard-coded SQL dialect list into registry. * feat(PMI-16): Dialect registry now uses scanning. * fix(PMI-16): Explicitly set report output encoding to avoid warning. * PMI-16: Added "local" directory to .gitignore * PMI-16: Got new SQL dialect registry running and improved logging. * PMI-16: Got properties-controlled dialects regsitry running. * PMI-16: Set fixed database version for CI build. Cleaned up integration test script. * PMI-16: Fixed quoting. * PMI-16: Corrected docker image version. * PMI-69: Fixed review findings of Andre Hacker. * PMI-69: Removed superfluous "@OverRide". * Pmi 75 sybase integration test (#36) * Rename EXASOL to Exasol * Update supported-dialects.md * SQL generation to use SQL standard "<>" for not-equal predicate * Support for native import from Oracle (issue #26) (#27) * Increment to version 1.0.1 for releasing * Increment version to 6.0.2-SNAPSHOT for next development iteration * Readme fixes (#29) * Display SQL statements, etc. in monospace and clean up syntax * Add newline after heading * Add script to run Exasol integration tests on Travis CI (#28) A new shell script `run_integration_tests.sh` executes the integration tests as defined in `integration-test-travis.yaml`. It uses the exasol/docker-db image to spin up an Exasol instance and execute the Exasol dialect integration tests. Travis CI automatically executes the test script for each new commit. * refactor(PMI-69): Turned hard-coded SQL dialect list into registry. * feat(PMI-16): Dialect registry now uses scanning. * fix(PMI-16): Explicitly set report output encoding to avoid warning. * PMI-16: Added "local" directory to .gitignore * PMI-16: Got new SQL dialect registry running and improved logging. * PMI-16: Got properties-controlled dialects regsitry running. * PMI-16: Set fixed database version for CI build. Cleaned up integration test script. * PMI-16: Fixed quoting. * PMI-16: Corrected docker image version. * Sybase Virtual Schema (#32) * Use SqlServer dialect as starting point * Add sybase dialect and override for ORDER BY * Setup Sybase integration tests: includes order by tests * Add WHERE test * Date/time types: handle conversion and add tests * Add sybase integer datatype tests * Add a whole bunch of data type tests * Add binary, varbinary, image and bit tests * Add Sybase to the supported dialects * Pmi 73 take over sybase from hendrik (#33) * PMI-16: Set fixed database version for CI build. Cleaned up integration test script. * PMI-16: Fixed quoting. * PMI-16: Corrected docker image version. * PMI-76: Cherry-picked integration test script improvements. * PMI-74: Used "mktemp". Improved configurability and readability. * PMI-74: Parameterized more hard-coded paths. * PMI-74: Change temp dir to rooted temp dir. * PMI-74: Improved readability by splitting file into functions. * PIM-74: Travis badge now points to default branch. * PMI-75: Adapted to new SQL dialect registry. * Added local and Scripts to .gitignore * Added .dbeaver* to .gitignore * PMI-75: Cleaned up and split documentation. * PMI-75: Fixed integration tests. Fixed database preparation scripts * PMI-75: Refactored IntegrationTestSetup.java for better readability and to reduce static methods to a minimum. * PMI-75: Got remote logging running. * PMI-75: Improved log message formatting. Added unit tests for custom formatter. * PMI-75: Set default log level explicitly. * PMI-75: Added fallback to STDOUT in case socket output stream is not available. * PMI-75: Improved documentation. * PMI-75: Removed distracting introduction. * Feature/pmi 91 update version to 1.1.0 (#37) * Deleted `increment-version.sh` script * Created `version.sh` script that has a verification and an updater mode * Introduced `product.version` property in master `pom.xml` * Used that property in all child-POM files * Used `version.sh` to update documentation * Added `version.sh verify` as build breaker in `.travis.yml`

redcatbear added 9 commits August 31, 2018 16:38

refactor(PMI-69): Turned hard-coded SQL dialect list into registry.

b72d4c2

feat(PMI-16): Dialect registry now uses scanning.

c0cc48f

fix(PMI-16): Explicitly set report output encoding to avoid warning.

d98caec

PMI-16: Added "local" directory to .gitignore

6a68579

PMI-16: Got new SQL dialect registry running and improved logging.

afd0b2b

PMI-16: Got properties-controlled dialects regsitry running.

28c0ed8

PMI-16: Set fixed database version for CI build. Cleaned up integrati…

7b5036f

…on test script.

PMI-16: Fixed quoting.

3911fe4

PMI-16: Corrected docker image version.

32fa492

andrehacker approved these changes Oct 1, 2018

View reviewed changes

redcatbear and others added 3 commits October 2, 2018 06:54

PMI-69: Fixed review findings of Andre Hacker.

599e180

Merge branch 'develop' into feature/PMI-69_Invert_dialect_dependencies

03dfd03

PMI-69: Removed superfluous "@OverRide".

df78690

redcatbear added the enhancement label Oct 2, 2018

redcatbear self-assigned this Oct 2, 2018

redcatbear merged commit 8c455ba into develop Oct 2, 2018

redcatbear deleted the feature/PMI-69_Invert_dialect_dependencies branch October 2, 2018 05:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/pmi 69 invert dialect dependencies #31

Feature/pmi 69 invert dialect dependencies #31

redcatbear commented Sep 7, 2018

andrehacker left a comment •

edited

Loading

andrehacker Oct 1, 2018

redcatbear Oct 2, 2018

andrehacker Oct 1, 2018

redcatbear Oct 2, 2018

andrehacker Oct 1, 2018

redcatbear Oct 2, 2018

andrehacker Oct 1, 2018

redcatbear Oct 2, 2018

andrehacker Oct 1, 2018

redcatbear Oct 2, 2018

Feature/pmi 69 invert dialect dependencies #31

Feature/pmi 69 invert dialect dependencies #31

Conversation

redcatbear commented Sep 7, 2018

andrehacker left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrehacker left a comment •

edited

Loading