Skip to content

Conversation

milanov
Copy link
Contributor

@milanov milanov commented Dec 14, 2019

I wasn't sure where to place exactly the AnsiDialect and the NonQuotingDialect in the tests, suggestions are welcome. Also the selectBuilder could be probably improved, as it does these instanceof checks, but that's what i was able to do without any major changes.

All database indentifiers, i.e. table names, column names and so on, now get quoted.
For most databases this means they get enclosed with double quotes.
For some databases this makes the the identifiers case sensitive.
In order to minimize the impact we convert identifiers their default letter casing.
This should be upper case according to the SQL standard but isn't for some databases.

The exact behavior regarding quoting and default letter casing gets controlled by a database specific `Dialect`.

Future changes will make the quoting of annotated columns and the default quoting behavior configurable.
@pivotal-issuemaster
Copy link

@milanov Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@milanov Thank you for signing the Contributor License Agreement!

@schauder
Copy link
Contributor

I had a quick look at it and it looks good so far. I'll do a proper review once the underlying PR is merged.

One good improvement would be to include the necessary changes to the documentation.

@milanov
Copy link
Contributor Author

milanov commented Dec 17, 2019

Thanks @schauder . I've identified the following docs that i should update:

@schauder
Copy link
Contributor

The first two documents/section are sufficient.

@milanov
Copy link
Contributor Author

milanov commented Dec 20, 2019

I've done the documentation changes locally and i'm now waiting for the underlying PR to be merged, so i can apply all my changes cleanly against master.

@mp911de mp911de closed this Jan 28, 2020
@mp911de
Copy link
Member

mp911de commented Jan 28, 2020

This PR was closed as it targeted an issue branch (spring-projects:issue/DATAJDBC-386) and not master. Please re-submit your changes against master.

@schauder
Copy link
Contributor

@mp911de I explicitly asked @milanov to make the PR based on the issue branch. I'd be fine reviewing it as it is.

@mp911de
Copy link
Member

mp911de commented Jan 28, 2020

The issue branch is now gone and the PR target cannot be changed anymore, hence my comment.

@milanov milanov deleted the issue/DATAJDBC-101 branch February 2, 2020 09:53
mp911de added a commit that referenced this pull request Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants