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

Fixed an issue where an unnecessary prefix was given when the random number was a column. #5179

Merged

Conversation

ytetsuro
Copy link
Contributor

@ytetsuro ytetsuro commented Oct 5, 2021

Fixed an issue where an unnecessary prefix was given when the random number was a column.

Description

The intention of this fix is because escaping is not necessary for RANDOM.

In ORACLE, random numbers are DBMS_RANDOM.RANDOM. That is, it refers to the RANDOM column of the DBMS_RANDOM table.
In MySQL, it is RAND(), it is function.

The third argument $escape of orderBy is null if it is omitted.
It depends on the protectIdentifiers property of Connection to toggle whether escaping is performed or not.
Also, the escaping process is to add the prefix to the table name.

In other words, in the case of ORACLE, it will add the prefix to DBMS_RANDOM.RANDOM and execute a query for a non-existent table, which will fail.

It is not a good idea for users to specify the value of the third argument escape with the driver in mind.

Therefore, I am making this fix because I thought it would be better to not escape for RANDOM and add the escaped column name to the randomKeyword property of each driver.

#2487 (comment)

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@ytetsuro ytetsuro mentioned this pull request Oct 5, 2021
5 tasks
@kenjis kenjis added the database Issues or pull requests that affect the database layer label Oct 6, 2021
Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

The strings for RANDOM are defined in the Builder classes.
We should define escaped/quoted string values, so that escaping is not needed.

@kenjis kenjis merged commit 8cc81bc into codeigniter4:develop Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Issues or pull requests that affect the database layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants