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.

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

Always name quote #209

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

ralfbecker
Copy link
Contributor

This pull-requests consists of two part:

a) new feature: always quote column and table names, instead of only if they contain special characters.
This allows to use reserved words of the database as column- or table-names eg. "group" or "timestamp".
I know this is a bad idea and we don't use that in our own software, but we have to create schemata for other projects we have no influence on too.

b) bug fix: PostgreSQL datadictionary queries columns and indexes from PostgreSQL's internal tables. If a table-name is quoted (either always because of above patch, or because it contains special chars!) the comparison will fail and no columns or indexes get reported.

As a workaround for a) we could already pass table- or column-name MySQL quoted eg. "group", which then get's unconditionally replaced by DB specific quotes in existing nameQuote implementation.
Then we would still run into bug fixed with b), which exists in current code-base independent of a).

Ralf

…column-names being reseved words like eg. "timestamp"

- fixing a bug in PostgreSQL driver, which becomes prominent with above fix, but not caused by it:
we must not nameQuote table names in ADODB_postgres64::MetaIndexes() as they are used in comparison to content of postgres system tables
…column-names being reseved words like eg. "timestamp"

- fixing a bug in PostgreSQL driver, which becomes prominent with above fix, but not caused by it:
we must not nameQuote table names in ADODB_postgres64::MetaIndexes/MetaColumns as they are used in comparison to content of postgres system tables
@ralfbecker
Copy link
Contributor Author

Any comment / opinion on that one?

@mnewnham
Copy link
Contributor

I agree totally with the principle of always quoting table names, In fact there is a much deeper issue with IBM DB2 where we use quoting to distinguish between tables named "TABLENAME" and "Tablename" in the the same database. I do have a substantial amount of code related to that issue that I have not yet committed that may conflict with your work, however I have not had time to review your submission.

You'll have to excuse delays in responding to your updates, the release of the new website and the associated work of updating documentation etc has had an impact on our workload associated with this project. We will get to them all eventually, and I would say that in principle, there is no reason that we would decline to add them to the project, so you can continue to work assuming your submitted features will be added to the project.

@ralfbecker
Copy link
Contributor Author

Just pushed an other bugfix around quoting of table names: datadict-mysql ADODB2_mysql::MetaColumns uses nameQuote on table-name before calling connection->MetaColumns, which uses "SHOW COLUMNS FROM %s".
This results in double quoting of table name, if nameQuote quotes the table name. This does not happen as often, if only certain table names get quoted (probably been overseen so far), but it causes the datadict method to fail, if we always quote.

Cleaner approach would be to move nameQuote into connection and call it in connections MetaColumns, instead of putting the quotes in the used SQL template.

Anyway current code with my fix produces always quoted table names in MetaColumns.

@mnewnham mnewnham self-requested a review November 26, 2019 23:30
Copy link
Contributor

@mnewnham mnewnham left a comment

Choose a reason for hiding this comment

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

@ralfbecker , I am currently reviewing all outstanding pull requests. This one seems to have 2 requests built into it

  1. A postgres bug fix
  2. An enhancement request

If you still believe these items are of value, could you break them into 2 and resubmit. If you have abandoned them please close them off.

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.

None yet

2 participants