Opened 9 years ago
Closed 9 years ago
#2851 closed defect (fixed)
v.in.ogr: imports empty values as empty strings instead of NULL values
| Reported by: | mlennert | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.0.4 |
| Component: | Default | Version: | svn-trunk |
| Keywords: | vector v.in.ogr null | Cc: | |
| CPU: | Unspecified | Platform: | Unspecified |
Description
v.in.ogr imports empty values in string fields (including date fields) as empty strings and not as NULL values.
Is there any specific reason for that ?
If not, I've attached a patch that changes that. But before applying, I would like to hear if anyone has an opinion on why this might or might not be a good idea.
Attachments (1)
Change History (10)
by , 9 years ago
| Attachment: | v_in_ogr_null_handling.diff added |
|---|
comment:1 by , 9 years ago
| Keywords: | vector v.in.ogr null added |
|---|---|
| Version: | unspecified → svn-trunk |
follow-up: 3 comment:2 by , 9 years ago
Although I am more purist and would say that NULL should be NULL, here's opinion why "no data" should be text (from Django framework):
"Avoid using null on string-based fields such as CharField and TextField because empty string values will always be stored as empty strings, not as NULL. If a string-based field has null=True, that means it has two possible values for “no data”: NULL, and the empty string. In most cases, it’s redundant to have two possible values for “no data;” the Django convention is to use the empty string, not NULL." https://docs.djangoproject.com/en/1.9/ref/models/fields/#null
If it comes down to voting, my vote goes for keeping NULL as NULL (do not alter data unless asked for).
comment:3 by , 9 years ago
Replying to marisn:
Although I am more purist and would say that NULL should be NULL, here's opinion why "no data" should be text (from Django framework):
"Avoid using null on string-based fields such as CharField and TextField because empty string values will always be stored as empty strings, not as NULL. If a string-based field has null=True, that means it has two possible values for “no data”: NULL, and the empty string. In most cases, it’s redundant to have two possible values for “no data;” the Django convention is to use the empty string, not NULL." https://docs.djangoproject.com/en/1.9/ref/models/fields/#null
If it comes down to voting, my vote goes for keeping NULL as NULL (do not alter data unless asked for).
I agree with you. NULL is a concept that is different from the empty string, even though in some cases they are equivalent (e.g. AFAIK there is no way to distinguish between the two in a csv file...).
In the case of this specific bug the issue is specifically for date fields which, IIUC, are only supported as such in OGR since 1.3 (but even Debian Squeeze, aka oldoldstable is beyond that version, so that should not be an issue). So a compromise solution could be to nullify the date and time fields (i.e. the first change in the patch), but not the string fields.
comment:4 by , 9 years ago
I've committed the change for both date and string types in trunk in r67516. That way this will get some testing. Not sure if this should already be backported to release70 or if we should wait for some more testing.
v.in.ogr imports empty values in string fields (including date fields) as empty strings and not as NULL values.
Is there any specific reason for that ?
If not, I've attached a patch against trunk that changes that. But before applying, I would like to hear if anyone has an opinion on why this might or might not be a good idea.