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

Information schema #25

Merged
merged 15 commits into from
Mar 28, 2023
Merged

Information schema #25

merged 15 commits into from
Mar 28, 2023

Conversation

OllieJones
Copy link
Contributor

#19 #20 #21 #22

Escaping LIKE literal % and _.
More information_schema and metadata compatiblity.
Unique constraint violations, and other errors when running queries, now detected and reported.
SUBSTRING and CHAR_LENGTH now handled by rewriting the query to SUBSTR and LENGTH.

@aristath aristath requested a review from adamziel March 22, 2023 11:52
if ( str_contains( $unescaped_value, '\_') || str_contains( $unescaped_value, '\%') ) {
$this->like_escape_count ++;
return str_replace(
array ( '\_', '\%' ),
Copy link
Collaborator

@adamziel adamziel Mar 23, 2023

Choose a reason for hiding this comment

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

Would _ be a wildcard in a string like \\_? If so, this may require a sub-parser just for the wildcards and backslashes. Lexer already does some of this work, though, to parse the string literals that may look like UPDATE x SET y='\\\'\\\\\\' – I wonder if that code is somehow reusable here.

Copy link
Collaborator

@adamziel adamziel Mar 23, 2023

Choose a reason for hiding this comment

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

Yeah, a double backslash turns _ into a wildcard again:

sqlite> select 1 WHERE '\\ot' LIKE '\\_%' ESCAPE '\';
1

Copy link
Collaborator

Choose a reason for hiding this comment

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

@OllieJones This was marked as resolved but the problem is still there.

@adamziel
Copy link
Collaborator

Great work on this one @OllieJones! I left some notes but this is the right direction. Do core unit tests still pass?

@OllieJones
Copy link
Contributor Author

By "core" unit tests I guess you mean the ones with this plugin. They do indeed pass.

I've added features and unit tests based on stuff I found in some data-maint plugins.

Thanks for your review; I'll incorporate your changes when I get a chance. LIKE '\_' and LIKE '\%' in my code are a nasty kludge.

@adamziel
Copy link
Collaborator

adamziel commented Mar 24, 2023

By "core" unit tests I guess you mean the ones with this plugin. They do indeed pass.

Oh I meant WordPress core with this plugin installed. I've got almost all of them to pass in #9 and it would be great to avoid regressions:

Tests: 14853, Assertions: 47545, Errors: 15, Failures: 37, Warnings: 34, Skipped: 60, Risky: 2.

The remaining failures can be mostly ignored as they're tests calling private wpdb mysql-related functions.

@aristath
Copy link
Member

I think we can merge this one as-is and continue iterating in follow-up PR.
What do you think @adamziel @OllieJones ?

@adamziel
Copy link
Collaborator

adamziel commented Mar 27, 2023

@aristath I'd still love to understand how it impacts WordPress core unit tests. What would it take to automate that with a CI check?

Also, I feels strongly about not counting like_expression_nesting separately – it would make the PR simpler = less code to maintain.

@OllieJones
Copy link
Contributor Author

OllieJones commented Mar 27, 2023

Fixes from your reviews now pushed. It's ready to merge, I think.

@aristath aristath merged commit 34d46b0 into WordPress:main Mar 28, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Mar 28, 2023
@aristath
Copy link
Member

Merged the PR, then fixed some CS issues in 65c50f4 and backported the changes to WordPress/wordpress-develop#3220 as well 👍

aristath added a commit to aristath/wordpress-develop that referenced this pull request Mar 29, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Mar 29, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Apr 5, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request May 8, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Jul 19, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Jul 20, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Jul 25, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Jul 31, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Aug 1, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Aug 4, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Aug 10, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Aug 24, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Aug 30, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Dec 28, 2023
aristath added a commit to aristath/wordpress-develop that referenced this pull request Jan 4, 2024
aristath added a commit to aristath/wordpress-develop that referenced this pull request Jan 9, 2024
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.

3 participants