-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancement/131 refactor metadata reading #153
Conversation
enhancement/121_Request_dispatching_and_remote_logging Conflicts: jdbc-adapter/pom.xml jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcAdapter.java jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/jdbc/JdbcAdapterTest.java
…/github.com/EXASOL/virtual-schemas into feature/124_Read_Redshift_Spectrum_metadata
enhancement/131_Refactor_metadata_reading Conflicts: jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcAdapter.java jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcMetadataReader.java
…ithub.com/EXASOL/virtual-schemas into enhancement/131_Refactor_metadata_reading
…c formatter settings
# Conflicts: # jdbc-adapter/pom.xml # jdbc-adapter/virtualschema-jdbc-adapter-dist/pom.xml # jdbc-adapter/virtualschema-jdbc-adapter/pom.xml # jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/impl/GenericSqlDialect.java # jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcAdapter.java # jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcMetadataReader.java # jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/CustomSqlGenerationVisitorTest.java # jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/FileBasedIntegrationTest.java # jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/SqlDialectTest.java # jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/impl/DialectTestData.java # jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/impl/OracleSqlDialectTest.java
…ithub.com/EXASOL/virtual-schemas into enhancement/131_Refactor_metadata_reading
integration tests.
…ithub.com/EXASOL/virtual-schemas into enhancement/131_Refactor_metadata_reading Conflicts: jdbc-adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/BaseRemoteMetadataReader.java jdbc-adapter/virtualschema-jdbc-adapter/src/test/java/com/exasol/adapter/jdbc/BaseRemoteMetadataReaderTest.java
...irtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/JdbcTypeDescription.java
Show resolved
Hide resolved
.../virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/SqlDialectFactory.java
Outdated
Show resolved
Hide resolved
...dbc-adapter/src/main/java/com/exasol/adapter/dialects/exasol/ExasolColumnMetadataReader.java
Show resolved
Hide resolved
...alschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialect.java
Outdated
Show resolved
Hide resolved
...alschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialect.java
Outdated
Show resolved
Hide resolved
...adapter/src/main/java/com/exasol/adapter/dialects/redshift/RedshiftColumnMetadataReader.java
Show resolved
Hide resolved
...hema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/redshift/RedshiftSqlDialect.java
Outdated
Show resolved
Hide resolved
...alschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/sybase/SybaseSqlDialect.java
Outdated
Show resolved
Hide resolved
...alschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/sybase/SybaseSqlDialect.java
Outdated
Show resolved
Hide resolved
...adapter/src/main/java/com/exasol/adapter/dialects/teradata/TeradataColumnMetadataReader.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the refactored version, it is much more modular now!
final VirtualSchemaAdapter adapter = new JdbcAdapter(); | ||
registerAdapterForSqlDialect(adapter, "DB2"); // FIXME: replace this hard-coded registration | ||
registerAdapterForSqlDialect(adapter, "EXASOL"); // FIXME: replace this hard-coded registration | ||
registerAdapterForSqlDialect(adapter, "GENERIC"); // FIXME: replace this hard-coded registration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HIVE dialect is not yet registered here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
...adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/SqlDialect.java
Outdated
Show resolved
Hide resolved
return new AdapterProperties(request.getSchemaMetadataInfo().getProperties()); | ||
} | ||
|
||
private SchemaMetadata readMetadata(final AdapterProperties properties, final ExaMetadata exasolMetadata) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still some helper methods in the JbdcAdapter class. I would like the JdbcAdapter just to implement the VirtualSchemaAdapter. Don't know if that is feasible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot actually. But I think we tackle those in the next refactoring round.
* #155: moved validation logic to dialects * Fixed integration tests errors
Looks good from my side! |
* Fixed identifier conversion
This is a major refactoring of the way SQL dialects and metadata reading are handled.