-
-
Notifications
You must be signed in to change notification settings - Fork 333
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 nonsensical cast_to_email and cast_to_uri functions #3564
Conversation
def create_date_casts(engine): | ||
type_body_map = _get_date_type_body_map() | ||
create_cast_functions(PostgresType.DATE, type_body_map, engine) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we create identical cast functions in create_datetime_casts()
, I've removed this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resulting cast functions look like the right ones. But, there's a problem.
You need to either:
- restore these, but make a different category for installing casts, or
- Use different logic in the record selector search function (see
db/records/operations/relevance.py
)
The problem is that if we merge this PR, you won't really be able to find records by searching email or URL columns.
def create_date_casts(engine): | ||
type_body_map = _get_date_type_body_map() | ||
create_cast_functions(PostgresType.DATE, type_body_map, engine) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
MathesarCustomType.URI, | ||
MathesarCustomType.EMAIL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing these with no other changes results in breaking searching for emails and URLs in the record selector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I see, thanks for pointing that out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this is good to go. Thanks!
Fixes #3555
Updated list of valid cast functions for
email
anduri
types:Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin