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.3] Change type of field "fieldparams" in table #_fields from text to mediumtext #44238

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

alikon
Copy link
Contributor

@alikon alikon commented Oct 12, 2024

Pull Request for Issue #43929 .

Summary of Changes

alter #__fields.fieldparams to MEDIUMTEXT

Testing Instructions

run the sql

ALTER TABLE `#__fields` MODIFY `fieldparams` MEDIUMTEXT NOT NULL;

or
https://artifacts.joomla.org/drone/joomla/joomla-cms/5.3-dev/44238/downloads/79556/

Actual result BEFORE applying this Pull Request

the field is TEXT

Expected result AFTER applying this Pull Request

the field is MEDIUMTEXT

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@alikon alikon changed the title mediumtext [5.3] Change type of field "fieldparams" in table #_fields from text to mediumtext Oct 12, 2024
@alikon alikon marked this pull request as ready for review October 12, 2024 07:07
@fgsw
Copy link

fgsw commented Oct 12, 2024

I have tested this item ✅ successfully on 03a0fab


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

…4-10-13.sql

Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@richard67
Copy link
Member

I've restored the previous human test result as the later changes which have invalidated the test counter were only some kind of code style without functional impact.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 8270392


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 13, 2024
@laoneo
Copy link
Member

laoneo commented Oct 14, 2024

Do we not need the changes on postgres too?

@richard67
Copy link
Member

richard67 commented Oct 14, 2024

Do we not need the changes on postgres too?

@laoneo No. PostgreSQL doesn’t know mediumtext or so, it uses TEXT which can be if unlimited length. See also PR #42606 .

@laoneo laoneo added this to the Joomla! 5.3.0 milestone Oct 14, 2024
@laoneo laoneo merged commit a5ace8d into joomla:5.3-dev Oct 14, 2024
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 14, 2024
@laoneo
Copy link
Member

laoneo commented Oct 14, 2024

Thanks!

@Fedik
Copy link
Member

Fedik commented Oct 14, 2024

Oh, hold on, why? It is mistake.
We should not encourage developers to store huge amount of data in parameters.
Extensions, modules, templates, menu, have the text type for parameters, why fields hould be diferent?

@laoneo
Copy link
Member

laoneo commented Oct 14, 2024

This here is more about end users and not extension developers as the fieldparams column contains the options from the UI.

@alikon alikon deleted the mediumtext branch October 14, 2024 09:38
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.

6 participants