-
Notifications
You must be signed in to change notification settings - Fork 42
Fix blank_nulls wrapping numeric CatalogNumber cast instead of raw column #7533
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
base: main
Are you sure you want to change the base?
Conversation
Gitesh307
left a comment
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.
Looks good! Verified on KU Birds, formatted catalog numbers now keep leading zeros in query results.
Iwantexpresso
left a comment
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.
Use the KUBrids database
Open the QB for CO and run a query with catalog number formatted.
- See that the query results for the formatted catalog number have leading zeros.
Tested on KuBirds as requested, and sd_paleo It looks like the issue has persisted, I also tested it on bohart_entomology's pinned insects collection and it is at least working fine there.
emenslin
left a comment
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.
- See that the query results for the formatted catalog number have leading zeros.
I tried testing using KUBirds based on the testing instructions, but it has a migration error when you open CO record format. I don't know if it impacts anything, however, when I tried uploading different versions of KUBirds they all seem to have this error. I decided to check another DB (KUfish) and I was able to recreate the issue and see that it was fixed there, so I think the PR is fine, but it might be best to test on DBs other than KUBirds.
11-25_09.46.mp4
grantfitzsimmons
left a comment
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.
- Use the KUBrids database
- Open the QB for CO and run a query with catalog number formatted.
- See that the query results for the formatted catalog number have leading zeros.
Do I need to test anything else?
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.
- See that the query results for the formatted catalog number have leading Zeroes
Just tested on ku birds and it seems to be fine now! I was not able to recreate my previous issues as well!

Sd paleo seems to be working as expected as well!

My apologies for requesting the changes even though the results are working fine!


Fixes #7510
Fixes #7549
Fixes issue found in v7.11.3 testing where on the KUBirds database, the CO catalog number formatted field was displaying results as numbers without leading zeros, like
1instead of this000000001.The issue introduced by recent refactoring of query-building defaults where the generated SQL for CatalogNumber fields wrapped the numeric cast expression inside IFNULL(...) rather than the raw column.
The ObjectFormatter.make_expr() method always wrapped the formatted expression in blank_nulls(...), even after _fieldformat() applied the numeric cast for CatalogNumber. After refactoring to use a fresh DefaultQueryFormatterProps(), the numeric_catalog_number flag was applied earlier, allowing the change in the generated SQL.
Here is the formatted catalog number in the query before:
And here it is after the changes in this PR:
Checklist
self-explanatory (or properly documented)
Testing instructions