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

[BUGFIX] Consistently integrate addWhereClause #539

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

tantegerda1
Copy link
Contributor

The addWhereClause is twice prefixed by a space and twice not. This
change makes all occurences consistently include a prefix space, thus
avoiding possibly invalid sql.

@tantegerda1
Copy link
Contributor Author

Other occurences, already prefixed with a space:

$row = $this->databaseConnection->exec_SELECTgetSingleRow(implode(',', $fieldList),
$configuration['table'],
$configuration['alias_field'] . '=' . $this->databaseConnection->fullQuoteStr($value, $configuration['table']) .
' ' . $configuration['addWhereClause']);

$row = $this->databaseConnection->exec_SELECTgetSingleRow(implode(',', $fieldList), $configuration['table'],
$configuration['id_field'] . '=' . $this->databaseConnection->fullQuoteStr($getVarValue, $configuration['table']) .
' ' . $configuration['addWhereClause']);

On the other hand, the wiki already states:

addWhereClause is an additional WHERE clause that must start with ' AND'.

(https://github.com/dmitryd/typo3-realurl/wiki/Configuration-reference#lookuptable)
so another option would be to consistently not include the space before the additional where-clause.

The current code poses a little trap, because it is not immediately obvious that you have a wrong configuration, since it works in some cases.

@tantegerda1
Copy link
Contributor Author

I'm happy to change the pull request into removing the additional space on every occurence if you like that better. I just recommend to avoid the inconsistency.

@dmitryd
Copy link
Owner

dmitryd commented Oct 5, 2017

Read the docs:

addWhereClause is an additional WHERE clause that must start with ' AND'. For example: ' AND deleted=0 AND hidden=0'.

@dmitryd
Copy link
Owner

dmitryd commented Oct 5, 2017

Basically the space should be in addWhereClause. The space in the code is probably left after rearranging or removing other conditions. It should be removed.

The addWhereClause was twice prefixed by a space and twice not. This
change makes all occurences consistently not include a prefix.
@tantegerda1 tantegerda1 force-pushed the bugfix-consistent-use-addwhere-integration branch from 3943e44 to e35ce9c Compare October 5, 2017 09:05
@tantegerda1
Copy link
Contributor Author

Changed the pull request to consistently not include a space.

@dmitryd
Copy link
Owner

dmitryd commented Apr 2, 2018

I like the change but I am afraid that there will be people who do not use space and will complain that their setup suddenly stopped working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants