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

Relation_type field too small #26605

Closed
erickgonzalez opened this issue Nov 6, 2023 · 3 comments · Fixed by #27140
Closed

Relation_type field too small #26605

erickgonzalez opened this issue Nov 6, 2023 · 3 comments · Fixed by #27140
Assignees
Labels

Comments

@erickgonzalez
Copy link
Contributor

Parent Issue

No response

Problem Statement

When creating a content type and a relationship field whose names are more than 35 chars, when trying to relate some content it fails.

Caused by: org.postgresql.util.PSQLException: ERROR: value too long for type character varying(64)
	at com.dotmarketing.factories.TreeFactory.insertTree(TreeFactory.java:346) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotmarketing.factories.TreeFactory.saveTree(TreeFactory.java:330) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.relateContent(ESContentletAPIImpl.java:4267) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.moveContentRelationships(ESContentletAPIImpl.java:6037) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.moveContentDependencies(ESContentletAPIImpl.java:6005) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.internalCheckin(ESContentletAPIImpl.java:4988) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.lambda$checkin$57(ESContentletAPIImpl.java:4505) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.concurrent.lock.StripedLockImpl.tryLock(StripedLockImpl.java:100) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.concurrent.lock.StripedLockImpl.tryLock(StripedLockImpl.java:55) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.concurrent.lock.IdentifierStripedLock.tryLock(IdentifierStripedLock.java:16) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.checkin(ESContentletAPIImpl.java:4504) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.checkin(ESContentletAPIImpl.java:8944) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotcms.content.elasticsearch.business.ESContentletAPIImpl.checkin(ESContentletAPIImpl.java:8988) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotmarketing.portlets.contentlet.business.ContentletAPIInterceptor.checkin(ContentletAPIInterceptor.java:184) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotmarketing.portlets.workflows.actionlet.SaveContentActionlet.executeAction(SaveContentActionlet.java:79) ~[dotcms_23.01.7_999999.jar:?]
	at com.dotmarketing.portlets.workflows.business.WorkflowAPIImpl.fireWorkflowPostCheckin(WorkflowAPIImpl.java:2411) ~[dotcms_23.01.7_999999.jar:?]

This is due the relation_type field is populated by parentContentTypeVelocityName.fieldVelocityName.

Steps to Reproduce

1- Create a Content-Type with the name AccountPageManageElection
2- Add a relationship field named withdrawalConfirmationDialogOnlyEnrolledInFuture to any other content type.
3- Try to create a contentlet.

Acceptance Criteria

Content Should be created successfully.

Let's create a UT that increases the varchar of the relation_type to 255.

dotCMS Version

23.01 +, and demo(23.10)

Proposed Objective

Customer Support

Proposed Priority

Priority 2 - Important

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.

https://dotcms.zendesk.com/agent/tickets/113860

Assumptions & Initiation Needs

No response

Quality Assurance Notes & Workarounds

No response

Sub-Tasks & Estimates

No response

@Sweater-Baron
Copy link

At least in Postgres, it's best to just make this text instead of varchar. There's no performance benefit to varchar over text, and I don't see a business logic reason to limit the length of this column at all.

https://www.postgresql.org/docs/current/datatype-character.html

There is no performance difference among these three types, apart from increased storage space when using the blank-padded type, and a few extra CPU cycles to check the length when storing into a length-constrained column. While character(n) has performance advantages in some other database systems, there is no such advantage in PostgreSQL; in fact character(n) is usually the slowest of the three because of its additional storage costs. In most situations text or character varying should be used instead.

@AndreyDotcms AndreyDotcms self-assigned this Jan 2, 2024
AndreyDotcms added a commit that referenced this issue Jan 3, 2024
AndreyDotcms added a commit that referenced this issue Jan 3, 2024
@erickgonzalez erickgonzalez linked a pull request Jan 3, 2024 that will close this issue
AndreyDotcms added a commit that referenced this issue Jan 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 4, 2024
* #26605 update column name and tests

* #26605 turning force run to true

* #26605 adding requested doc
@erickgonzalez erickgonzalez reopened this Jan 4, 2024
@erickgonzalez
Copy link
Contributor Author

Content was created successfully, tested using a fresh install and also an upgrade.

@josemejias11
Copy link

Approved: Tested on master_8aae521, Docker, macOS 13.0, FF v121.0.1

@erickgonzalez erickgonzalez added LTS : Next Ticket that will be added to LTS and removed Triage labels Jan 19, 2024
@erickgonzalez erickgonzalez added Next LTS Release and removed LTS : Next Ticket that will be added to LTS labels Feb 13, 2024
erickgonzalez added a commit that referenced this issue Feb 14, 2024
@erickgonzalez erickgonzalez added the Release : 23.01.12 Included in LTS patch release 23.01.12 label Feb 15, 2024
erickgonzalez added a commit that referenced this issue Feb 16, 2024
@erickgonzalez erickgonzalez added Release : 23.10.24 v4 Included in LTS patch release 23.10.24 v4 and removed Next LTS Release labels Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants