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

[5.1] Fix comment syntax in update SQL scripts "5.1.0-2024-02-24.sql" for adding TUF #43306

Merged

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Apr 18, 2024

Pull Request for Issue #43302 .

Summary of Changes

This pull request (PR) fixes the syntax error in the SQL code comment in the update SQL scripts "5.1.0-2024-02-24.sql" mentioned in the referred issue.

Note that the SQL syntax error will never happen when updating Joomla to 5.1.x as the updater splits the SQL script into the single statements, stripping off any comments (also those with the syntax error), and runs these statements one by one. The error happens only if you copy the content of the script into an SQL client to run the statements "manually", e.g. when trying to fix an update which failed for other reasons.

Testing Instructions

Code review.

Or if you want to do a real test: Depending on which database type you have available for testing (MySQL/MariaDB or Postgresql), copy the content of the update SQL script "5.1.0-2024-02-24.sql" into an SQL client, e.g. phpMyAdmin for MySQL/MariaDB or phpPgAdmin for PostgreSQL, replace the #__ by your real table name prefix and run the SQL statement.

  • With MySQL/MariaDB use script "administrator/components/com_admin/sql/updates/mysql/5.1.0-2024-02-24.sql".
  • With PostgreSQL use script "administrator/components/com_admin/sql/updates/postgresql/5.1.0-2024-02-24.sql".

Actual result BEFORE applying this Pull Request

See issue #43302 .

Expected result AFTER applying this Pull Request

No SQL syntax error with that comment.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

@exlemor
Copy link

exlemor commented Apr 21, 2024

Hi @richard67,

I must be doing something wrong because whether the patch is Applied or Not:

If I use the Import feature in phpmyadmin, the 5.1.0-2024-02-24.sql is successfully upload/merged/imported without errors.

If I use the SQL queries window, I get an error as long as the comment blocks are there whether the patch is Applied or Not,
if I remove the comment blocks, the code works fine (which obviously defeats the purpose of this test).

MySQL 8.0.36.

(sorry) :(

@richard67
Copy link
Member Author

richard67 commented Apr 21, 2024

@exlemor You are right, there is one more syntax error in another comment line. Will fix in a few minutes.

Update: No, the PR is ok.

@richard67
Copy link
Member Author

@exlemor As my instructions say you should use the SQL input window, not the import. What I've forgotten to mention is that it needs to replace the #__ by the real database prefix. I've added that to my testing instructions.

It does not really need to apply the PR for testing. It is enough to copy the original SQL from the unmodified file in the 5.1-dev branch and from the modified SQL file from this PR here by going to the files on GitHub.

@richard67
Copy link
Member Author

richard67 commented Apr 21, 2024

@exlemor P.S. I've just tested here and it works as I described. I.e. with the original SQL from 5.1-dev I get the syntax error mentioned in the issue because of the two ----------------------------------------------------------- comment lines, and with the file from this PR these two comment lines become -- -------------------------------------------------------- (mind the space after the first 2 -), and there is no syntax error anymore.

The unmodified files can be found here:

The files from this PR are here:

@dautrich
Copy link

I have tested this item ✅ successfully on 2b1c2af

Tested with:

@alikon
Copy link
Contributor

alikon commented Apr 22, 2024

I have tested this item ✅ successfully on 2b1c2af


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43306.

@alikon
Copy link
Contributor

alikon commented Apr 22, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43306.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 22, 2024
@wilsonge wilsonge merged commit 6f2dff1 into joomla:5.1-dev Apr 23, 2024
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 23, 2024
@wilsonge wilsonge added this to the Joomla! 5.1.1 milestone Apr 23, 2024
@richard67 richard67 deleted the 5.1-dev-update-sql-comment-syntax branch April 23, 2024 12:57
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.

None yet

6 participants