-
-
Notifications
You must be signed in to change notification settings - Fork 383
Improve inference for numeric columns #4406
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
Conversation
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.
LGTM!
@@ -17,7 +17,7 @@ if [[ $EXIT_CODE -eq 0 ]]; then | |||
for i in {1..50}; do | |||
pg_isready -U mathesar -d mathesar_testing && break || sleep 0.5 | |||
done | |||
pg_prove --runtests -U mathesar -d mathesar_testing "$@" | |||
pg_prove --runtests -U mathesar -d mathesar_testing -v "$@" |
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.
I like this change, are you aware of a way to make only the failing tests verbose?
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.
I asked the same question, but couldn't find a way.
Co-authored-by: Pavish Kumar Ramani Gopal <[email protected]>
@pavish I accepted your suggestion as per our call. |
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
Related to #4370
This greatly improves the accuracy, safety, and performance of type inference for numeric columns.
Technical details
Previously, we simply tried to cast a sample of the column to
numeric
using ourmsar.cast_to_numeric(text)
function. This regularly resulted in false negatives, and potentially resulted in false positives. Now, we:numeric
).This PR does not make necessary changes to the front end code to enable all possible performance benefits of the back end changes. To get those benefits, the front end should submit the returned
details.decimal_p
anddetails.group_sep
values back to the API to enable the faster casting behavior when actually confirming and saving the table.TODO
In follow-up PRs, we should:
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin