Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support for unquoted Postgres identifiers (fix for #74) #75

Merged
merged 12 commits into from
Feb 12, 2019
Merged

Support for unquoted Postgres identifiers (fix for #74) #75

merged 12 commits into from
Feb 12, 2019

Conversation

snehlsen
Copy link
Contributor

Unqouted identifiers are supported for the postgres dialect.
An error is raised during virtual schema creation (or refresh) if postgres identifiers contain uppercase characters. Schema creation can be enforced by setting the ignore_error_list property to POSTGRES_IGNORE_UPPERCASE_TABLES.

@snehlsen
Copy link
Contributor Author

@redcatbear @andrehacker : Please review, since this change introduces a generalized error handling approach and a lot of naming decisions.

andrehacker
andrehacker previously requested changes Feb 4, 2019
View reviewed changes
Copy link
Contributor

@andrehacker andrehacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have some remarks, makes sense to talk about this.

.../virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcAdapterProperties.java Outdated Show resolved Hide resolved
.../virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/jdbc/JdbcAdapterProperties.java Show resolved Hide resolved
redcatbear
redcatbear requested changes Feb 4, 2019
View reviewed changes
Copy link
Collaborator

@redcatbear redcatbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with findings.

...virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/AbstractSqlDialect.java Outdated Show resolved Hide resolved
...adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/SqlDialect.java Outdated Show resolved Hide resolved
...adapter/virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/SqlDialect.java Show resolved Hide resolved
...virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/AbstractSqlDialect.java Show resolved Hide resolved
...virtualschema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/AbstractSqlDialect.java Show resolved Hide resolved
...ma-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/impl/PostgreSQLSqlDialectTest.java Show resolved Hide resolved
...ma-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/impl/PostgreSQLSqlDialectTest.java Outdated Show resolved Hide resolved
...ma-jdbc-adapter/src/test/java/com/exasol/adapter/dialects/impl/PostgreSQLSqlDialectTest.java Outdated Show resolved Hide resolved
...tualschema-jdbc-adapter/src/test/java/com/exasol/adapter/jdbc/JdbcAdapterPropertiesTest.java Outdated Show resolved Hide resolved
...tualschema-jdbc-adapter/src/test/java/com/exasol/adapter/jdbc/JdbcAdapterPropertiesTest.java Show resolved Hide resolved
@snehlsen
Copy link
Contributor Author

snehlsen commented Feb 4, 2019

Thanks for the review! I will first unifiy the two error handling properties and add consistency checks.
Then add documentation for the new property (readme.md + postgresql.md) and go through @redcatbear suggestions.

redcatbear
redcatbear approved these changes Feb 7, 2019
View reviewed changes
Copy link
Collaborator

@redcatbear redcatbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as issue tickets exist for the findings where the refactorings are postponed, I am fine.

@snehlsen
Copy link
Contributor Author

Final naming is: IGNORE_ERRORS='POSTGRESQL_UPPERCASE_TABLES'
We added consistency checks and documentation.

@snehlsen snehlsen merged commit 76aefa9 into develop Feb 12, 2019
@snehlsen snehlsen deleted the postgres_identifiers branch February 12, 2019 09:33
andrehacker
andrehacker reviewed Feb 12, 2019
View reviewed changes
Copy link
Contributor

@andrehacker andrehacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made some comments, feel free to merge consider or not (i.e. approved)

jdbc-adapter/doc/sql_dialects/postgresql.md Show resolved Hide resolved
jdbc-adapter/doc/sql_dialects/postgresql.md Show resolved Hide resolved
...schema-jdbc-adapter/src/main/java/com/exasol/adapter/dialects/impl/PostgreSQLSqlDialect.java Show resolved Hide resolved
@snehlsen snehlsen mentioned this pull request Feb 12, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrehacker andrehacker andrehacker left review comments

@redcatbear redcatbear redcatbear approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@snehlsen @redcatbear @andrehacker