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

Remove redundant casting functions #4333

Open
Tracked by #4370
Anish9901 opened this issue Mar 13, 2025 · 3 comments
Open
Tracked by #4370

Remove redundant casting functions #4333

Anish9901 opened this issue Mar 13, 2025 · 3 comments
Labels
affects: architecture Improvements or additions to architecture affects: technical debt Improves the state of the codebase help wanted Community contributors can implement this ready Ready for implementation type: maintenance work: backend Related to Python, Django, and simple SQL
Milestone

Comments

@Anish9901
Copy link
Member

Description

We have a lot of casting functions in 45_msar_type_casting.sql which are redundant.
E.g. Casting to uuid doesn't require 3 function with different parameter types overloaded for each string like type. A casting function withtext param can handle all string types including text, character and varchar. This might also be true for other types numeric but needs to be thoroughly tested.

mathesar=# CREATE OR REPLACE FUNCTION msar.cast_to_uuid(text)
RETURNS uuid
AS $$
BEGIN
  RETURN $1::uuid;
END;
$$ LANGUAGE plpgsql RETURNS NULL ON NULL INPUT;
CREATE FUNCTION

mathesar=# select msar.cast_to_uuid('bc2166dd-ca1d-41c6-9c66-610f844ca139');
             cast_to_uuid             
--------------------------------------
 bc2166dd-ca1d-41c6-9c66-610f844ca139
(1 row)

mathesar=# select msar.cast_to_uuid('bc2166dd-ca1d-41c6-9c66-610f844ca139'::char(36));
             cast_to_uuid             
--------------------------------------
 bc2166dd-ca1d-41c6-9c66-610f844ca139
(1 row)

mathesar=# select msar.cast_to_uuid('bc2166dd-ca1d-41c6-9c66-610f844ca139'::varchar);
             cast_to_uuid             
--------------------------------------
 bc2166dd-ca1d-41c6-9c66-610f844ca139
(1 row)

Expected behavior

We should ideally remove all of string like varients of casting functions and only keep their text version. If there is a possibility to do the same for non string like types as well we should consider that.

@Anish9901 Anish9901 added affects: architecture Improvements or additions to architecture affects: technical debt Improves the state of the codebase good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. ready Ready for implementation work: backend Related to Python, Django, and simple SQL labels Mar 13, 2025
@Anish9901 Anish9901 added this to the High priority milestone Mar 13, 2025
@zackkrida zackkrida added type: maintenance help wanted Community contributors can implement this and removed good first issue Everything in "Help wanted", PLUS being relatively easy and straightforward to implement. labels Mar 18, 2025
@zackkrida zackkrida modified the milestones: High priority, v0.2.3 Mar 19, 2025
@Anand1923
Copy link

@Anish9901 Is this an open issue? I would like to take it up.

@zackkrida
Copy link
Member

Hi @Anand1923 this issue is open and marked as help wanted so feel free to work on it. You can reach out to @Anish9901 here if you have any questions while implementing. Thank you!

@Anish9901
Copy link
Member Author

@Anand1923 If you work on this, please try to create multiple small PRs(e.g. 1 PR for 2-3 cast type) instead of one really big PR with a lot of deletions, this way it would be easy for us to review and merge your PRs quicker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: architecture Improvements or additions to architecture affects: technical debt Improves the state of the codebase help wanted Community contributors can implement this ready Ready for implementation type: maintenance work: backend Related to Python, Django, and simple SQL
Projects
None yet
Development

No branches or pull requests

3 participants